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

Enriched mapping documentation #741

Merged
merged 6 commits into from
Apr 27, 2024
Merged

Enriched mapping documentation #741

merged 6 commits into from
Apr 27, 2024

Conversation

cmungall
Copy link
Collaborator

@cmungall cmungall commented Apr 25, 2024

  • added a Commands/Mappings notebook
  • extended the OAK mappings guide to talk about some difficulties around directionality
  • clarified how weights work in lexmatch rules
  • removed duplicate synonymizer section in rules data model (it now has its own data model)

@cmungall cmungall requested review from matentzn and gouttegd April 25, 2024 18:05
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 82.22222% with 32 lines in your changes are missing coverage. Please review.

Project coverage is 74.74%. Comparing base (8a6dfc3) to head (e6195e3).
Report is 4 commits behind head on main.

Files Patch % Lines
src/oaklib/datamodels/mapping_rules_datamodel.py 82.22% 32 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #741      +/-   ##
==========================================
- Coverage   75.54%   74.74%   -0.81%     
==========================================
  Files         275      278       +3     
  Lines       32190    32758     +568     
==========================================
+ Hits        24317    24484     +167     
- Misses       7873     8274     +401     

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

Copy link
Contributor

@gouttegd gouttegd left a comment

Choose a reason for hiding this comment

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

Looks good to me, apart from a confusing statement about the directionality of mappings (see embedded comment).

docs/guide/mappings.rst Outdated Show resolved Hide resolved
@gouttegd
Copy link
Contributor

gouttegd commented Apr 26, 2024

Is the mappings command supposed to only be used with the SQLite adapter (as in the examples provided in the new notebook)?

Because when I try to use it directly on a OBO file, it gives a completely different output:

$ runoak -i uberon.obo mappings UBERON:0000995
This ontology interface contains 267 prefixes:
  AAO -> http://purl.obolibrary.org/obo/AAO_
  ADO -> http://purl.obolibrary.org/obo/ADO_
  ADW -> http://purl.obolibrary.org/obo/ADW_
  AEO -> http://purl.obolibrary.org/obo/AEO_
[…]

It goes on to list all the prefixes, but never actually lists any mapping.

Whereas using it through the SQLite adapter gives the same results as in the doc:

$ runoak -i sqlite:obo:uberon mappings UBERON:0000995
subject_id: UBERON:0000995
predicate_id: oio:hasDbXref
object_id: BTO:0001424
mapping_justification: semapv:UnspecifiedMatching
subject_label: uterus
subject_source: UBERON
object_source: BTO
[…]

Is that a bug or the expected behaviour? If the latter, this warrants a mention in the doc.

@cmungall cmungall merged commit ac3409e into main Apr 27, 2024
9 checks passed
@cmungall
Copy link
Collaborator Author

Hmm, on my phone now so can't check now but I believe that behavior is when pronto can't parse the obo

- Some mapping predicates such as ``skos:broadMatch`` and ``skos:narrowMatch`` are inherently directed, with the meaining inverted in the opposite direction
- Many mappings are only be asserted in one ontology, and may be "invisible" when queried from the mapped ontology

At the time of writing, most ontologies bundle mappings that are uncommitted `oio:hasDbXref`.
Copy link
Collaborator

@twhetzel twhetzel Apr 27, 2024

Choose a reason for hiding this comment

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

What does "bundle mappings" mean?

uncommitted?

Is the prefix oio mentioned earlier or 110% known by readers what this is?


While mappings are often thought of as bidirectional, this is not quite true in practice:

- Some mapping predicates such as ``skos:broadMatch`` and ``skos:narrowMatch`` are inherently directed, with the meaining inverted in the opposite direction
Copy link
Collaborator

Choose a reason for hiding this comment

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

directed --> directional?

this with the meaining inverted in the opposite direction may not be very clear and also just noticed typo in meaining

While mappings are often thought of as bidirectional, this is not quite true in practice:

- Some mapping predicates such as ``skos:broadMatch`` and ``skos:narrowMatch`` are inherently directed, with the meaining inverted in the opposite direction
- Many mappings are only be asserted in one ontology, and may be "invisible" when queried from the mapped ontology
Copy link
Collaborator

Choose a reason for hiding this comment

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

Might want to expand on why mappings may be "invisible" from an ontology, e.g. each ontology must agree to the mapping and include the mapping and actively assert the mapping to the term in the other ontology for it to be visible from either ontology.

@@ -74,61 +80,43 @@ classes:
attributes:
subject_source_one_of:
multivalued: true
description: The source of the subject to be matched.
Multiple values can be provided, it must match at least one.
Copy link
Collaborator

Choose a reason for hiding this comment

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

reading only from the gh diff, an example value looks like it would be useful to add

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.

4 participants