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

Demonstrate using TAXRANK for ranks and properties #120

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cthoyt
Copy link
Contributor

@cthoyt cthoyt commented Nov 27, 2024

Closes #96, closes #97

This PR does the following:

  1. Replaces the ad-hoc NCBITaxon identifiers for tax ranks with terms defined in the TAXRANK ontology
  2. Replaces the ad-hoc NCBITaxon relation ncbitaxon:has_rank with TAXRANK:1000000
  3. Extends missing tax ranks using terms defined in the TAXRANK ontology
  4. Explicitly handles the no rank rank

It depends on phenoscape/taxrank#5, where I added the remaining 12 ranks that weren't already represented. That PR was merged and released on November 28th, 2024. Related: that PR included adding a tax rank for "strain", which was the point of discussion also in #107

Copy link
Contributor

@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.

I personally very much like this change, and I approve of it, but I will request changes so that it is not accidentally merged before I can get some community feedback (and we can use my disapproval as a blocker until the taxrank PR is released).

Thank you very much!

@jplfaria
Copy link

@matentzn, are there any updates on "community feedback" for this change? Is this issue the best place to provide feedback?

I have been working to create ontology descriptions of other taxonomy sources (gtdb and silva) where I include TAXRANK per @cthoyt suggestion. Using TAXRANK to define taxonomic ranks will vastly facilitate comparison and mapping between different taxonomies.

@cmungall
Copy link
Member

cmungall commented Feb 17, 2025 via email

@jamesaoverton
Copy link
Collaborator

jamesaoverton commented Feb 18, 2025

We discussed this in the OBO Community Slack on November 27, but unfortunately the discussion did not get back here.

The current version of this PR drops the ncbitaxon:has_rank predicate and classes that we've been using for 20+ years, and so breaks any system that queries for or displays the old ranks.

If the PR just added new TAXRANK stuff without dropping the old stuff, it wouldn't be a breaking change.

@cmungall
Copy link
Member

I think having an interim period with both would be fine, but can we start a survey of people who are using the old properties with a view to declaring EOL?

We use ncbitaxon heavily but this change would not break things for us AFAICT, but others use cases may differ

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants