-
Notifications
You must be signed in to change notification settings - Fork 21
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
updated fao_class description link and permissible values #726
updated fao_class description link and permissible values #726
Conversation
Updated FaoClassEnum to match available list
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 know what the MIxS policy is on removing permissible values - these may have been used in existing projects.
I would instead mark these as deprecated; e.g
Lithosols:
deprecated: No longer in FAO as of YYYY-MM-DD / version X
src/mixs/schema/mixs.yaml
Outdated
Ferralsols: | ||
Fluvisols: | ||
Gleysols: | ||
Greyzems: | ||
deprecated: true, value no longer recognized by FAO, mixs/issues/696 |
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 would prefer that the issue link is a real github url in a see also
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 am going to push back a bit on the "I would prefer" .. there's no standard structure for this process. I would prefer deprecated
be a boolean or structured & there be a reason
attribute so someone doesn't need to understand how see-also
is a reason for deprecation. Because that's not intuitive to me. But, that's a LinkML limitation.
So how / who gets to say when someones preference is the action to take?
I am happy to change the issue path to the ACTUAL issue URL, seems long and like it's saying the same thing to me. But, excessive clarity makes sense.
This comment was marked as resolved.
This comment was marked as resolved.
To discuss at TWG: deprecation protocol in place. However, policy should still be discussed & by that I mean I have tagged these to be deprecated but I'm not sure we've settled on WHEN things get removed from the mixs.yaml and put into the deprecated.yaml? |
Hello Montana,I was not on the last call.Can you please clarify why the allowable values are being changed ?Much appreciated,LynnSent from my iPhoneOn Jul 17, 2024, at 6:08 PM, Montana ***@***.***> wrote:
To discuss at TWG:
deprecation protocol in place. However, policy should still be discussed & by that I mean I have tagged these to be deprecated but I'm not sure we've settled on WHEN things get removed from the mixs.yaml and put into the deprecated.yaml?
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>
|
Hello Montana, Another consideration: it would be useful to put a term review / update protocol in place. So that if the TWG/CIG are considering changing the terms of a checklist or package, in a substantial way, that the process of the review include the original developers/CIG members involved with the development. That way, we are not mistakenly changing what the standards was meant to collect.In the past the CIG did not edit packages/checklists, other than to fix minor edits.We need to consider if this is an area of work we want to do or should be doing as it has been the community developers that have decided what metadata terms they want in their standards.Cheers,LynnSent from my iPhoneOn Jul 17, 2024, at 6:08 PM, Montana ***@***.***> wrote:
To discuss at TWG:
deprecation protocol in place. However, policy should still be discussed & by that I mean I have tagged these to be deprecated but I'm not sure we've settled on WHEN things get removed from the mixs.yaml and put into the deprecated.yaml?
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>
|
Good point @lschriml . @mslarae13 and Sierra have put together a good, tracable deprecation framework but I don' think we have something equivalent for traceable changes yet. |
Or, specifically in this case, where do deprecated permissible values go? Other schema elements like classes and slots are eventually moved https://github.com/GenomicsStandardsConsortium/mixs/blob/main/src/mixs/schema/deprecated.yaml. Permissible values can't be moved (or searched) in a free standing manner |
There is not yet, a protocol for deprecating elements of a metadata term.
Let's discuss at an upcoming call.
I think this could fit into a change log, type of reporting.
…On Thu, Jul 18, 2024 at 10:53 AM Mark Andrew Miller < ***@***.***> wrote:
Or, specifically in this case, where do deprecated permissible values go?
Other schema elements like classes and slots are eventually moved
https://github.com/GenomicsStandardsConsortium/mixs/blob/main/src/mixs/schema/deprecated.yaml.
Permissible values can't be moved (or searched) in a free standing manner
—
Reply to this email directly, view it on GitHub
<#726 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABBB4DNKA7R5GLREDUIXZXDZM7JE7AVCNFSM6AAAAABAM5BI2KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMZWG44DEMRUGA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
--
Lynn M. Schriml, Ph.D.
Associate Professor
Institute for Genome Sciences
University of Maryland School of Medicine
Department of Epidemiology and Public Health
670 W. Baltimore St., HSFIII, Room 3061
Baltimore, MD 21201
P: 410-706-6776 | F: 410-706-6756
***@***.***
|
In the issue linked above. #696, and @only1chunts comment #696 (comment) The FAO permissible values have changes & the previously provided link is no longer active. So I updated the enum to match the updated list of values. But yes, we can discuss |
Sound good.Sent from my iPhoneOn Jul 19, 2024, at 6:18 PM, Montana ***@***.***> wrote:
@lschriml
In the issue linked above. #696, and @only1chunts comment #696 (comment)
The FAO permissible values have changes & the previously provided link is no longer active. So I updated the enum to match the updated list of values. But yes, we can discuss
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Thanks @cmungall Based on the workflow proposed by Sierra and approved byt GSC and NMDC, the terms are marked for deprecation for 1 release. And after a release as marked for deprecation, the terms/items marked for deprecation are then moved into the deprecated.yaml. As such, for this PR, the terms only get marked for deprecation. Does this satisfy your request? |
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.
Based on discussion in today's Technical Working Group meeting, I approve this PR. Thanks @mslarae13.
I have started a new issue about URLs and links going in uri/cuire range slots, like see_also
I get the sense that review from @cmungall may need to be re-requested. In my opinion, this PR satisfies his concern.
Additional note based on todays discussion, the current deprecation method doesn't not have a way to capture JUST the permissible values within an enum. A LinkML update will be needed to move these deprecated PVs into the |
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.
Looks great, thanks!
…-site merge in main
Updated FaoClassEnum to match available list
Updated fao_class description to have updated link & reference for enum source