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

add permissible values to InstrumentVendorEnum and InstrumentModelEnum for sequencing applications #70

Merged
merged 7 commits into from
Jan 17, 2024

Conversation

aclum
Copy link
Collaborator

@aclum aclum commented Jan 17, 2024

This PR expands on the two enumerations to support sequencing based vendors and models.

We will need to write migration code to map existing values of instrument_name to vendor and model.

Adding to InstrumentVendorEnum and InstrumentModelEnum for sequencing applications.
fix typo.
@aclum aclum added the migration-needed After applying this schema change, some valid data may become invalid. Data migration is required. label Jan 17, 2024
changing to a valid model
updating to a valid permissible value for model.
@aclum aclum marked this pull request as ready for review January 17, 2024 01:57
Copy link
Member

@turbomam turbomam left a comment

Choose a reason for hiding this comment

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

This is an excellent enumeration PR. Thanks for the thorough annotation with OBI terms. Was it a pain to pick them all out? @cmungall and I have some ideas for providing some automation.

If there's one single exact_mapping for a permissible value, it would be better to express it as the meaning of the PV. mappings are textual annotations but meanings are structural. If the schema or the data are converted to RDF, the meanings can be queried for subclasses, superclasses, and other grouping axioms.

If that's OK with you, it could be quickly accomplished with

s/exact_mappings:\n +- /meaning: /

Update exact_mappings to meaning for InstrumentVendorEnum and InstrumentModelEnum, add revio as permissible value for InstrumentModelEnum.
@aclum
Copy link
Collaborator Author

aclum commented Jan 17, 2024

@turbomam This was a bit painful b/c you can't see the actual term IDs in the tree view. Probably would have been better to dump this and use some of the ontology tools but I'm not very familiar with those. It would be useful to have a little script that could make a permissible value out of every child of a term (in this case a good starting point would have been to make a term for all the children of OBI:0400103, then delete the ones we don't need).

Add prefix for BAO
src/schema/nmdc.yaml Outdated Show resolved Hide resolved
@aclum aclum requested a review from turbomam January 17, 2024 18:34
Switching back to an  OBI for illumina permissible value meaning
@aclum aclum merged commit 462750a into main Jan 17, 2024
2 checks passed
@aclum aclum deleted the 1566-sequencing-vendor-and-models branch January 17, 2024 22:00
@brynnz22 brynnz22 added the migrator-overhaul-needed The current migrator design is not sufficient to tackle the migration label Jan 30, 2024
@brynnz22 brynnz22 changed the title 1566- add permissible values to InstrumentVendorEnum and InstrumentModelEnum for sequencing applications add permissible values to InstrumentVendorEnum and InstrumentModelEnum for sequencing applications Mar 27, 2024
@brynnz22 brynnz22 added the migrator-written a migrator script has been written addressing the PR/schema change label Apr 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
migration-needed After applying this schema change, some valid data may become invalid. Data migration is required. migrator-overhaul-needed The current migrator design is not sufficient to tackle the migration migrator-written a migrator script has been written addressing the PR/schema change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

expand nmdc.yaml InstrumentModel to have additional instruments and. the multiple obitraps in the enum
4 participants