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

Patch language tags #295

Merged
merged 7 commits into from
Feb 20, 2025
Merged

Patch language tags #295

merged 7 commits into from
Feb 20, 2025

Conversation

jamesaoverton
Copy link
Member

@jamesaoverton jamesaoverton commented Feb 14, 2025

Addresses #293

This PR explicitly sets literals to use either @en (preferred) or xsd:string (when that's what the source uses).

  • switch to @en annotations to all COB terms
  • use @en for all annotations in the cob-edit.tsv template
  • override non-COB literals as required to match source ontologies
    • patch-labels.ru
    • patch-definitions.ru
  • rework product creation to suit these changes
  • cross-check against source ontologies
    • use OntoFox to import small numbers of terms from some large ontologies
    • merge and run robot report

There are still 12 label errors from make crosscheck, because at the COB workshop we decided to override these labels (implemented in #288):

ENVO:01000813 astronomical body part@en VS geophysical entity@en
GO:0003674 gene product or complex activity@en VS molecular_function
GO:0008150 biological process@en VS biological_process
GO:0110165 cellular anatomical entity VS cellular anatomical structure
MOP:0000543 molecular process VS physico-chemical process
NCBITaxon:131567 cellular organism VS cellular organisms

Judging by the problems we had with this language tag issue, I guess that very few people actually tested #288.

@sebastianduesing
Copy link
Collaborator

YES!!! Thanks, James. I just tested this on OBI. With this version of COB plugged into the OBI-COB transition, make all no longer throws any multiple label violations, and only three multiple definition violations remain, and they're all violations where there's an actual (subtle enough that I hadn't caught it before) difference between the text of the definitions:

  • One of these, an extra space in OBI's version of the definition of OBI:0000295 'is specified input of' that's not in COB's version of the definition, should get fixed in OBI, I believe.
  • The other two are OBI:0000293 'has specified input' and OBI:0000299 'has specified output'. These both have OBI definitions of the form "The inverse property of is specified input of"/"The inverse property of is specified output of". The COB versions of both of these definitions use the underscores that used to be used in the labels of 'is specified input/output of'. I believe these should get fixed in COB.

I tested updating the definitions in obi-edit.owl to align with the COB definitions and still got a multiple labels violation, which I believe came from the remaining xsd:string vs @en issue. But if I'm understanding this PR correctly, I believe that when the definitions are corrected so that they do actually match, updating the files in this PR will fix that language tag issue as it fixed the issue for all of the label/definition conflicts that already matched.

I can make PRs as needed to update the definitions for those three terms. James, I'm not sure if you'd prefer to add a commit to this PR to fix the definitions of the two that need to be updated in COB, or if I should create a separate PR to fix those.

$(TMPDIR)/merged_imports.owl: $(X_IMPORT_OWL_FILES)
$(ROBOT) merge \
$(foreach X,$^,--input $(X)) \
remove --term IAO:0000115 \
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 removes too much. I'll fix it when I'm back to work on Tuesday.

@jamesaoverton
Copy link
Member Author

Thanks @sebastianduesing, I'll get back to you on Tuesday.

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.

Two quick Qs for context, I will review again!

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 went through it now in more detail, and and looks good to me; I left a few comments here and there but nothing that would change my mind on the approval.

@jamesaoverton jamesaoverton marked this pull request as ready for review February 18, 2025 19:49
@jamesaoverton
Copy link
Member Author

I addressed @matentzn's points. @sebastianduesing: I want to update OBI to match what's here in COB, so I'm ignoring the remaining problems with OBI definitions. There are a few other definitions that are failing the crosscheck test, but we'll work on them separately.

This PR is a clear improvement on the last release, so I plan to merge it and make a new release tomorrow.

@sebastianduesing
Copy link
Collaborator

That sounds good. I'm happy to move forward with this PR and make those changes in OBI.

@jamesaoverton jamesaoverton merged commit 6b2c1e8 into master Feb 20, 2025
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