-
Notifications
You must be signed in to change notification settings - Fork 22
current land use enum with URL safe characters #972
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
base: main
Are you sure you want to change the base?
Conversation
LinkML Linting ResultsSummary
Problems per Schema/home/runner/work/mixs/mixs/src/mixs/schema/mixs.yamlErrors
|
|
src/mixs/schema/mixs.yaml
Outdated
cities: | ||
conifers: | ||
examples: | ||
- value: cypress |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably want to keep "conifers: pine"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TIL the practice of adding examples on PV's! ✅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think PVs should ever have examples
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will switch all examples to PVs of their own and assert is_a
between them (hemlock) and their super-classes (conifer)
As discussed we need to enumerate these longer lists as option values: Instead of having them listed as 'examples' in LinkML. |
Thanks @lschriml Could you please look though these observed values
I would like to analyze how much of the hierarchy should be captured in MIxS per se And reach out to the original contributors of this term to see what kinds of revisions they are comfortable with vs what really needs to be left as is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome job turning the string_serialization
on cur_land_use
to a enumeration. I added a few comments on if/how we can possibly further enrich the CurLandUseEnum
(by adding meanings)?
src/mixs/schema/mixs.yaml
Outdated
cities: | ||
conifers: | ||
examples: | ||
- value: cypress |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TIL the practice of adding examples on PV's! ✅
@@ -25,6 +25,99 @@ subsets: | |||
nucleic acid sequence source: | |||
investigation: | |||
enums: | |||
CurLandUseEnum: | |||
permissible_values: | |||
badlands: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add meanings to some of these PV's?
For example, is this an appropriate meaning for the badlands PV? http://purl.obolibrary.org/obo/ENVO_00000127
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a really good idea @sujaypatil96
@cmungall and I discussed it today. Chris suggested focusing our PV meaning efforts on
- enums where virtually all of the PVs would have a mapping to a well-maintained ontology
- slots that are already being frequently used in INSDC Biosamples
I don't think 2 is true here, and I think 1 might be very difficult to accomplish because the current land use PVs are a mixture of land uses (which might be in EnvO) and organisms that might be found on land being used in some way. The organism names are a mixture of singular and plural, and may have many to many mappings within NCBI taxonomy
So I am going to skip PV meaning sin the PR
CurLandUseEnum: | ||
permissible_values: | ||
badlands: | ||
cities: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly, meaning for cities – http://purl.obolibrary.org/obo/ENVO_00000856
Looks like there's a merge conflict that needs resolution on this PR @turbomam? |
LinkML Linting ResultsSummary
Problems per Schema/home/runner/work/mixs/mixs/src/mixs/schema/mixs.yamlErrors
|
No description provided.