-
Notifications
You must be signed in to change notification settings - Fork 128
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
Update ms.yml #1014
base: master
Are you sure you want to change the base?
Update ms.yml #1014
Conversation
config/ms.yml
Outdated
replacement: https://github.com/HUPO-PSI/psi-ms-CV/releases/download/v | ||
tests: | ||
- from: /4.1.186/ms.owl | ||
to: https://github.com/HUPO-PSI/psi-ms-CV/releases/download/v4.1.86/ms.owl |
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.
The tests are failing because "4.1.186" != "4.1.86". I assume that's a typo.
# intercept exact matches before version prefix blindly grabs everything | ||
- exact: /ms.owl | ||
replacement: https://github.com/HUPO-PSI/psi-ms-CV/releases/latest/download/ms.owl | ||
- exact: /psi-ms.owl |
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 the right way to add "psi-ms.owl", which will resolve to http://purl.obolibrary.org/obo/ms/psi-ms.owl.
config/ms.yml
Outdated
@@ -4,7 +4,8 @@ idspace: MS | |||
base_url: /obo/ms | |||
|
|||
products: | |||
- ms.owl: https://github.com/HUPO-PSI/psi-ms-CV/releases/latest/download/psi-ms.owl | |||
- ms.owl: https://github.com/HUPO-PSI/psi-ms-CV/releases/latest/download/ms.owl | |||
- psi-ms.owl: https://github.com/HUPO-PSI/psi-ms-CV/releases/latest/download/psi-ms.owl |
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.
These products must all be of the form "id_space.file_extension". The ID_SPACEs are governed by OBO, and the relevant one here is "MS"/"ms". You're not allowed to add a new ID_SPACE here with "psi-ms.owl".
@@ -13,20 +14,33 @@ example_terms: | |||
- MS_0000000 | |||
|
|||
entries: | |||
- prefix: /releases/ |
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.
PURLs are supposed to be permanent. You can't just drop all these /releases/
PURLs.
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.
It's safer to put all prefix
entries at the bottom, after all the exact
entries.
Hi @mobiusklein. Thanks for the PR -- I know it can be daunting. I made some comments. Maybe look at examples in this repo for other ontologies that you're familiar with. If you get stuck, let me know. Can you confirm for me in some way that you have authority to make these changes? A note from @germa (the official contact listed here https://obofoundry.org/ontology/ms.html) would be one way to let me know. |
I made some, if not all the requested changes, and fixed the typo in the tests, but wanted to see if the tests would fail still. @germa hasn't been involved in the PSI-MS CV's maintenance for a long time. I've been a maintainer for over two years, and was the contact person for two previous issues here: |
This is an attempt to solve HUPO-PSI/psi-ms-CV#259 without rewriting how our OWL files are generated.
It keeps the names the same, but should now resolve versioned pURLs.
I am unable to setup a test VM at the moment, so I couldn't test how the blind
prefix: /
rule would interact withproducts
. From running thetranslate_yaml.py
script, it didn't seem likeproducts
produced a rule in the generated.htaccess
file. If this would introduce a conflict, they can be removed?