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

runoak diff erroneously names "relationships" --> "mappings" in markdown summary #735

Closed
matentzn opened this issue Apr 7, 2024 · 4 comments · Fixed by #750
Closed

runoak diff erroneously names "relationships" --> "mappings" in markdown summary #735

matentzn opened this issue Apr 7, 2024 · 4 comments · Fixed by #750
Assignees

Comments

@matentzn
Copy link
Contributor

matentzn commented Apr 7, 2024

The "Relation ship added" section is erroneously called "Mappings added".

Example: obophenotype/human-phenotype-ontology#10446 (comment)

reports/difference_release_base.md: tmp/hp-released.obo $(KGCL_ONTOLOGY)
	runoak -i simpleobo:tmp/hp-released.obo diff -X simpleobo:$(KGCL_ONTOLOGY) -o $@ --output-type md
@hrshdhgd
Copy link
Collaborator

hrshdhgd commented Apr 9, 2024

So there might be some misunderstanding on my part or a bug in code. The table created under Mappings added are derived from the Change object EdgeCreation. This is designed from oaklib's simple_mappings_by_curie() output. So it should be mappings(?) or relationships?

Similarly

  • Mappings removed: EdgeDeletion
  • Mappings changed: EdgeChange

Code from differ_interface:

# ! Mappings
self_mappings = set(self.simple_mappings_by_curie(entity))
other_mappings = set(other_ontology.simple_mappings_by_curie(entity))
mappings_added_set = other_mappings - self_mappings
mappings_removed_set = self_mappings - other_mappings
mapping_changed_set = _find_mapping_changes(self_mappings, other_mappings)
if mappings_added_set:
for mapping in mappings_added_set:
predicate, xref = mapping
edge_created = EdgeCreation(
id=_gen_id(),
subject=entity,
predicate=predicate,
object=xref,
)
yield edge_created
if mappings_removed_set:
for mapping in mappings_removed_set:
predicate, xref = mapping
deleted_edge = EdgeDeletion(
id=_gen_id(),
subject=entity,
predicate=predicate,
object=xref,
)
yield deleted_edge
if mapping_changed_set:
for changes in mapping_changed_set:
object, new_predicate, old_predicate = changes
edge_change = EdgeChange(
id=_gen_id(),
about_edge=Edge(subject=entity, predicate=old_predicate, object=object),
old_value=old_predicate,
new_value=new_predicate,
)
yield edge_change

Code from simpleobo:

elif tag == TAG_RELATIONSHIP:
rels1 = stanza1.pair_values(TAG_RELATIONSHIP)
rels2 = stanza2.pair_values(TAG_RELATIONSHIP)
for p, v in rels1:
p_curie = self.map_shorthand_to_curie(p)
if (p, v) not in rels2:
yield kgcl.EdgeDeletion(id=_id(), subject=t1id, predicate=p_curie, object=v)
for p, v in rels2:
p_curie = self.map_shorthand_to_curie(p)
if (p, v) not in rels1:
yield kgcl.EdgeCreation(id=_id(), subject=t2id, predicate=p_curie, object=v)

map_shorthand_to_curie() uses simple_mappings_by_curie()

def map_shorthand_to_curie(self, rel_code: PRED_CODE) -> PRED_CURIE:
"""
Maps either a true relationship type CURIE or a shorthand packages to a CURIE.
See `section 5.9 <https://owlcollab.github.io/oboformat/doc/obo-syntax.html#5.9>`_
:param rel_code:
:return:
"""
for _, x in self.simple_mappings_by_curie(rel_code):
if x.startswith("BFO:") or x.startswith("RO:"):
return x
if ":" not in rel_code and ":" in x:
return x
return rel_code

Thoughts @cmungall

@cmungall
Copy link
Collaborator

edge is very generic in KGCL, it encompasses relationships and mappings

@cmungall
Copy link
Collaborator

OK, I misspoke. edges is KGCL are the same as edges in OAK

this code is not quite right:

            if mappings_added_set:
                for mapping in mappings_added_set:
                    predicate, xref = mapping
                    edge_created = EdgeCreation(
                        id=_gen_id(),
                        subject=entity,
                        predicate=predicate,
                        object=xref,
                    )
                    yield edge_created
            if mappings_removed_set:
                for mapping in mappings_removed_set:
                    predicate, xref = mapping
                    deleted_edge = EdgeDeletion(
                        id=_gen_id(),
                        subject=entity,
                        predicate=predicate,
                        object=xref,
                    )
                    yield deleted_edge
            if mapping_changed_set:
                for changes in mapping_changed_set:
                    object, new_predicate, old_predicate = changes
                    edge_change = EdgeChange(
                        id=_gen_id(),
                        about_edge=Edge(subject=entity, predicate=old_predicate, object=object),
                        old_value=old_predicate,
                        new_value=new_predicate,
                    )

                    yield edge_change

simply change edge_ to mapping_ and Edge* to Mapping* to get the correct KGCL types

@hrshdhgd
Copy link
Collaborator

Fixed by #775

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