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

Remodeling of range for analyte_cateogry #2249

Conversation

samobermiller
Copy link
Contributor

@samobermiller samobermiller commented Nov 7, 2024

This PR replaces AnalyteCategoryEnum with NucelotideSequencingEnum and MassSpectrometryEnum and adds titles to the corresponding permissible values
fixes #2154

…m) that inherit from AnalyteCategoryEnum minus certain invalid PVs (ex NucleotideSequencingEnum has no PV metaproteome). assigned NucleotideSequencing and MassSpectrometry subclasses these enums in analyte_category. added two new tests to invalid folder (NucleotideSequencing-working.yaml and MassSpectrometry-working.yaml) which should allow test to pass because they use nonpermissible values (metaproteome and metagenome respectively) in analyte_category. BUT test is failing when these are in the invalid folder and passing when they're in the valid folder. need to figure out why
Copy link

github-actions bot commented Nov 7, 2024

PR Preview Action v1.4.8
🚀 Deployed preview to https://microbiomedata.github.io/nmdc-schema/pr-preview/pr-2249/
on branch gh-pages at 2024-12-07 00:41 UTC

…into-types-allowable-for-nucleotidesequencing-and-massspectrometry
@kheal
Copy link
Contributor

kheal commented Dec 6, 2024

@aclum and @turbomam FYI, neither @samobermiller or I could actually get the inheritance on the "sub Enums" to result in properly validating or invalidating examples (see this example src/data/invalid/MassSpectrometry-working.yaml in this branch validates but it should not).

We tried a few different things to no avail. We only opened this PR to see if the documentation would render appropriately (it doesn't). Just to give you a heads up about this branch and PR. I don't think @samobermiller or I plan to pick it back up anytime soon (we were using it as a training example for how to write and test example yaml data).

@aclum
Copy link
Contributor

aclum commented Dec 6, 2024

This does not appear to be working properly based on the passing invalid data MassSpectrometry-working.yaml, it should fail b/c of an invalid analyte_category PV.

@aclum
Copy link
Contributor

aclum commented Dec 6, 2024

@kheal yes, I see that now.

@aclum
Copy link
Contributor

aclum commented Dec 6, 2024

cc @pkalita-lbl in case you have any linkml insights

@pkalita-lbl
Copy link
Collaborator

The JSON Schema generator only handles explicitly defined permissible values at the moment: https://github.com/linkml/linkml/blob/5cc5a16e345d3ee5233a193eba4909a8999b43db/linkml/generators/jsonschemagen.py#L426-L457

So, in particular, it doesn't have any logic yet to deal with inherits or minus as used in this changes.

@aclum aclum marked this pull request as ready for review December 7, 2024 00:17
@aclum aclum marked this pull request as draft December 7, 2024 00:22
@aclum aclum marked this pull request as ready for review December 7, 2024 00:33
@aclum aclum changed the title update PVs for subclasses NucleotideSequencing and MassSpectrometry, post berkeley Remodeling of enumerations for analyte_cateogry Dec 7, 2024
@aclum aclum changed the title Remodeling of enumerations for analyte_cateogry Remodeling of range for analyte_cateogry Dec 7, 2024
@aclum
Copy link
Contributor

aclum commented Dec 13, 2024

I will wait until the 12/16 release to merge this in but @turbomam @sierra-moxon @kheal @mslarae13 I need at least one reviewer on this

@@ -413,7 +413,7 @@ slots:
description: >
The type of analyte(s) that were measured in the data generation process and analyzed
in the Workflow Chain
range: AnalyteCategoryEnum

Copy link
Member

Choose a reason for hiding this comment

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

👍 - so for now, we fall back to "string" for the range of analyte_category if not otherwise defined in slot_usage.

@aclum aclum merged commit 4b01a37 into main Dec 17, 2024
3 checks passed
@aclum aclum deleted the 2154-berkeley-breakdown-analytecategoryenum-into-types-allowable-for-nucleotidesequencing-and-massspectrometry branch December 17, 2024 00:59
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.

berkeley breakdown AnalyteCategoryEnum into types allowable for NucleotideSequencing and MassSpectrometry
6 participants