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

Mock run of new (proposed) SOP noctua-model replacement and updates #483

Open
kltm opened this issue May 12, 2022 · 9 comments
Open

Mock run of new (proposed) SOP noctua-model replacement and updates #483

kltm opened this issue May 12, 2022 · 9 comments

Comments

@kltm
Copy link
Member

kltm commented May 12, 2022

This is to be a standing ticket to support and discuss mock runs of current and proposed SOPs for https://github.com/orgs/geneontology/projects/87 . The reason to consolidate is to 1) make it look more like how it will be run in the future and 2) prevent the proliferation of expensive branches of noctua-models. Currently, we'll be testing:

Replaces #479 and #476 .

@kltm
Copy link
Member Author

kltm commented May 12, 2022

@vanaukenk It would be good to decide on a standard commit message for relation updates.

@kltm
Copy link
Member Author

kltm commented May 12, 2022

@balhoff In your example given at https://github.com/geneontology/minerva/pull/478/files#diff-aefb43779e951b6f961416f1195b5d9b753eca8a12a29f64a7a2cd8fb432e63dR226, you have a file class-replacements.tsv. where/what is this file? Is this for future use? (When testing, it seems to be required.)

@vanaukenk
Copy link

@kltm I'm thinking that it might be useful to have a generic commit message for any changes that are not made using a 'replaced by' tag in an ontology.

How about: "Automated change YYYY-MM-DD: CURIE:xxxxxxx updated to CURIE:yyyyyyy" ?

@kltm
Copy link
Member Author

kltm commented May 12, 2022

@vanaukenk We can't really have variables in a git commit message in this case as it is for the entire code tree at that moment in time, rather than individual files, so individual CURIEs would not make much sense.

@vanaukenk
Copy link

@kltm - I thought you were referring to the actual comment in the models (where I'm still wondering if we should use 'replaced by' for these relations updates since that has a specific meaning in ontologies.. @balhoff - any thoughts?)

For a git commit message, then, how about:
'Updating RO relations as specified in noctua-models/migration/replace_relations.tsv'

I am open to suggestions here, if there's something you and @balhoff think would be more informative or appropriate.

@kltm
Copy link
Member Author

kltm commented May 12, 2022

@vanaukenk Sounds good to me. I think my only comment would be something a step outward: geneontology/noctua-models#233 (comment)

kltm added a commit to geneontology/noctua-models that referenced this issue May 12, 2022
@balhoff
Copy link
Member

balhoff commented May 13, 2022

@balhoff In your example given at https://github.com/geneontology/minerva/pull/478/files#diff-aefb43779e951b6f961416f1195b5d9b753eca8a12a29f64a7a2cd8fb432e63dR226, you have a file class-replacements.tsv. where/what is this file? Is this for future use? (When testing, it seems to be required.)

Yes that is to be able to replace classes as well as relations. We should just create an empty file alongside the relations one for now.

@kltm
Copy link
Member Author

kltm commented May 13, 2022

Thank you for the clarification @balhoff ; I've updated the SOP.
The changes connected with just a relations change are available here: https://github.com/geneontology/noctua-models/commits/issue-minerva-483-test-updates

@kltm
Copy link
Member Author

kltm commented Jun 23, 2022

@vanaukenk A replaced_by changes set is available here: https://github.com/geneontology/noctua-models/commits/issue-minerva-483-test-updates
(I'm mostly sure I did this right, but there was some fiddliness.)

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

No branches or pull requests

3 participants