Remove ChemicalEntity as range for metabolite_identified slot
#235
Conversation
kheal
changed the title
ChemicalEntity as range for metabolite_identified slot
|
…aff) groups=20(staff),12(everyone),61(localaccounts),79(_appserverusr),80(admin),81(_appserveradm),98(_lpadmin),701(com.apple.sharepoint.group.1),33(_appstore),100(_lpoperator),204(_developer),250(_analyticsusers),395(com.apple.access_ftp),398(com.apple.access_screensharing),101(com.apple.access_ssh-disabled),400(com.apple.access_remote_ae) slot for class
|
@sierra-moxon @turbomam @mslarae13 - welcome back from GSC. Please review when you are able. |
| id: | ||
| required: true | ||
| structured_pattern: | ||
| syntax: "{id_nmdc_prefix}:chem-{id_shoulder}-{id_blade}$" |
There was a problem hiding this comment.
this may need to change to a ChEBI prefix pattern per our discussions with Chris
| @@ -1592,6 +1593,7 @@ slots: | |||
| inlined_as_list: true | |||
|
|
|||
| known_as: | |||
| description: An identifier of a ChemicalEntity. | |||
There was a problem hiding this comment.
It would be helpful for me to have a more detailed description here; in my (albeit strange) brain, "known_as" typically refers to a "name" or "alias" vs. an identifier...this could be tangential to this PR though! happy to refine description that in a subsequent PR.
There was a problem hiding this comment.
I agree. I find the name of this slot unintuitive and frankly problematic for the reason you point out. It was introduced during the Berkeley refactoring and there are no data in mongo using this slot, so if you can help to think of a more appropriate slot name and description, that would be much appreciated! I'd be happy to roll that into this PR.
There was a problem hiding this comment.
The only domain of the slot known_as is the class PortionOfSubstance. I'd advocate to make a better slot name and description before we use it (aka, now ;) ).
|
@sierra-moxon I'm going to move this back to draft until we settle on ID structure! |
|
Is the invalid example data file invalid for a single reason? Please include a super brief explanation for the invalidity in the filename, like |
Description
This PR will:
ChemicalEntityas range for metabolite_identified slot.metabolite_identifiedslot should not beChemicalEntitynmdc-schema#2153, as discussed here: `ChemicalEntity` use cases, questions, guidance. nmdc-schema#2151structured_patternconstraint and requirement onidslot forChemicalEntityclass and invalid test for bad typecodestructured_patternconstraint onknown_asslot to match its range ofChemicalEntityand adds description forknown_asslotknown_asslot needs description andstructured_patternconstraints nmdc-schema#2121ChemicalEntityclass - @sierra-moxon or @turbomam please confirm this is appropriate as this is my first exposure to that slot.Reviewers
to review: @mslarae13, @turbomam, @sierra-moxon
to tag for schema freeze purposes: @aclum, @mbthornton-lbl, @corilo, @SamuelPurvine, @naglepuff, @sujaypatil96, @brynnz22, @eecavanna
PR Information
What type of PR is this? (check all applicable)
slotslot(made it wide open!)Did you add/update any tests?
Could this schema change make it so any valid data becomes invalid?
ChemicalEntitybecause there is nochemical_entity_setuntil the berkeley schema, therefore no way for there to beChemicalEntitys in mongo)Does this PR have any downstream implications?