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 high level classification #131

Merged
merged 3 commits into from
Dec 7, 2023
Merged

Conversation

katiermullen
Copy link
Collaborator

Address #129

The inferred breed hierarchy is classifying correctly except for the South American camelid breeds - Llama breed, Alpaca breed, Vicuna breed and Guanaco breed should be children of South American camelid breed.

@katiermullen katiermullen marked this pull request as draft November 27, 2023 16:15
@sabrinatoro
Copy link
Collaborator

Great job @katiermullen

  • please see comments in the high-level classification spreadsheet
  • we need to add "vertebrate breeds" as a "has ontology root term" for the ontology (instead of "vertebrata ". We can do this together so you see what I mean
  • The cat component was not refreshed, so we cannot see how the classification was updated.
  • Some of the high-level classification terms should be under another high-level classification terms (eg. 'Ass breed' and 'horse breed' should be under 'equid breed'
  • Note to ourselves: once we havereviewed the cat, we should update all breeds and add the "vertebrate breed" parent

@katiermullen
Copy link
Collaborator Author

@sabrinatoro thank you for your review!

  • please see comments in the high-level classification spreadsheet

done! A couple of questions remain for us to address on a call.

  • we need to add "vertebrate breeds" as a "has ontology root term" for the ontology (instead of "vertebrata ". We can do this together so you see what I mean

Looking forward to it.

  • The cat component was not refreshed, so we cannot see how the classification was updated.

Will update and refresh.

  • Some of the high-level classification terms should be under another high-level classification terms (eg. 'Ass breed' and 'horse breed' should be under 'equid breed'

Can I make the SC of the 'Ass breed' and 'Horse breed' be 'Equid breed' VBO:0400033? This way, the high level classification will be apparent in the inferred and asserted views of the ontology. Or is there another way I should address this for Equid, Bird, Poultry breed, etc?

  • Note to ourselves: once we have reviewed the cat, we should update all breeds and add the "vertebrate breed" parent

can't wait!

Address #131 - added high level classification terms and updated ontology root term.

This is a work in progress and some known issues are outlined here #135

Please note, this PR also contains changes to the ontology that address #133 #120 #133
@katiermullen katiermullen marked this pull request as ready for review December 2, 2023 00:16
@katiermullen
Copy link
Collaborator Author

@matentzn could you please review this PR with emphasis on the high level classification and update to the ontology root term.

@sabrinatoro has kindly offered to review the other changes that this PR encompasses.

Thank you!!!

Copy link
Member

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

There is too much to review here effectively but too notes:

  1. The logical equivalence axiom for "vertebrate breed" is circular and should be removed
  2. The human definition of "South American camelid breed" is "Vertebrate breeds including Llama, Guanaco, Vicuña and Alpaca breeds." That is not a good definition, at best a "description". A definition should always roughly follow the "genus+differentiae" model. The word "including" is a real risk for a definition as the enumeration is expected to be exhaustive.
    What other specific questions do you have for me?

Improve definitions, add ECs, and new high level classifications for duck breed, pheasant breed and partridge breed.
@katiermullen
Copy link
Collaborator Author

@matentzn Thank you for your review comments and further explanations today! Much appreciated.

Copy link
Collaborator

@sabrinatoro sabrinatoro left a comment

Choose a reason for hiding this comment

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

This is fantastic job, @katiermullen !!!
Thank you!

I will merge and ask Nico to update the monarch VBO OLS instance

@sabrinatoro sabrinatoro merged commit e65e471 into master Dec 7, 2023
1 check passed
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.

3 participants