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

Add case variant synonyms to Orphanet and ORDO #1003

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

cthoyt
Copy link
Member

@cthoyt cthoyt commented Nov 28, 2023

This is an attempt to reconcile Orphanet and ORDO, which have weirdly overlapping synonyms. It's still not clear if these should be merged into a single namespace, or reorganized in a different way

Copy link

codecov bot commented Nov 28, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (ab3fd30) 40.82% compared to head (9339784) 40.82%.
Report is 6 commits behind head on main.

❗ Current head 9339784 differs from pull request most recent head 59ced11. Consider uploading reports for the commit 59ced11 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1003   +/-   ##
=======================================
  Coverage   40.82%   40.82%           
=======================================
  Files         138      138           
  Lines        7928     7928           
  Branches     1847     1847           
=======================================
  Hits         3237     3237           
  Misses       4489     4489           
  Partials      202      202           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@matentzn matentzn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, how will you now deal with the Orphanet identifiers and their uris?

@cthoyt
Copy link
Member Author

cthoyt commented Nov 29, 2023

@matentzn haven't figured this out yet, just seeing to what extent I can add case variant synonyms without breaking anything

@cthoyt cthoyt changed the title Update ORDO Add case variant synonyms to Orphanet and ORDO Nov 29, 2023
@@ -73514,7 +73512,7 @@
"version": "4.3",
"version.iri": "https://www.orphadata.com/data/ontologies/ordo/last_version/ORDO_en_4.3.owl"
},
"pattern": "^C?\\d+$",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this change, you wont validate any "normal" orpha codes, they don't have that "C"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was on purpose. The goal of the bioregistry is to give a summary of all of the semantic spaces. Just because it turns out that Orphanet smashed two semantic spaces into the same URI format string, it doesn't mean we can't describe them as accurately as possible.

Like David said, Orphanet's ORDO vocabulary is for the properties. The rest is regular Orphanet

Copy link
Collaborator

@matentzn matentzn Nov 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes but like I noted, it make more practical sense to push them together instead of having two, else how will you be able to say that both have the same uri_prefix?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So far I have not heard a clear argument why the separation is necessary. I have good reasons why it is not necessary. I can only think of a single reason, which is being able to redirect xrefs from browsers to separate destinations depending on whether they are properties or diseases. Can you point me to the issue that clearly explains that this is what you are trying to solve?

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

Successfully merging this pull request may close these issues.

2 participants