Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use nmdc-schema-provided function to get typecode from slot pattern #878

Conversation

eecavanna
Copy link
Collaborator

@eecavanna eecavanna commented Jan 23, 2025

On this branch, I changed how the minter gets typecodes from the schema.

Details

Previously, the minter would use a locally-defined function (named extract_typecode_from_pattern) to extract typecodes from id slot patterns. That ran the risk of falling out of sync with how the same thing is done in the nmdc-schema package. On this branch, I deleted the locally-defined function and updated the calling code to call an equivalent function (named get_typecode_for_future_ids) provided by the nmdc-schema package.

Related issue(s)

Fixes #866

Related subsystem(s)

  • Runtime API (except the Minter)
  • Minter
  • Dagster
  • Project documentation (in the docs directory)
  • Translators (metadata ingest pipelines)
  • MongoDB migrations
  • Other

Testing

  • I tested these changes (explain below)
  • I did not test these changes

I tested these changes by implementing a basic doctest and running it locally via:

docker compose run --rm --no-deps fastapi sh -c 'pip install nmdc-schema==11.3.0 && python -m doctest -v nmdc_runtime/minter/config.py'

Note: I included pip install nmdc-schema==11.3.0 && in the command because I am currently unable to rebuild my fastapi container via make up-dev. This is a separate problem, predating this PR branch.

🙋 Question: Does the GHA test process run doctests?

Documentation

  • I have not checked for relevant documentation yet (e.g. in the docs directory)
  • I have updated all relevant documentation so it will remain accurate
  • Other (explain below)

Maintainability

  • Every Python function I defined includes a docstring (test functions are exempt from this)
  • Every Python function parameter I introduced includes a type hint (e.g. study_id: str)
  • All "to do" or "fix me" Python comments I added begin with either # TODO or # FIXME
  • I used black to format all the Python files I created/modified
  • The PR title is in the imperative mood (e.g. "Do X") and not the declarative mood (e.g. "Does X" or "Did X")

Comment on lines +29 to +33
# Tests #2 and #3: We get only the typecode we expect, for a class whose pattern contains multiple typecodes.
>>> any((td["name"] == "dgms" and td["schema_class"] == "nmdc:MassSpectrometry") for td in typecode_descriptors)
True
>>> any((td["name"] == "omprc" and td["schema_class"] == "nmdc:MassSpectrometry") for td in typecode_descriptors)
False
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: The schema docs show this pattern:

slot_usage:
  id:
    name: id
    structured_pattern:
      syntax: '{id_nmdc_prefix}:(dgms|omprc)-{id_shoulder}-{id_blade}$'

The first typecode in the (...|...) portion is, indeed, the one our team members want the minter to use when minting new IDs for instances of the MassSpectrometry class.

Copy link
Collaborator

@dwinston dwinston left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dwinston
Copy link
Collaborator

Question: Does the GHA test process run doctests?

I don't think so.

@eecavanna
Copy link
Collaborator Author

Thanks for reviewing this, @dwinston. I will merge it in now.

I will file a ticket about updating the GHA-run test runner(s) to also find and run doctests.

@eecavanna eecavanna merged commit b4db574 into main Jan 23, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use nmdc-schema-provided function to extract typecode from id regex pattern
2 participants