NCATSTranslator / TranslatorArchitecture

MIT License
9 stars 11 forks source link

Adding principal regarding querying/returning predicate inverses. #38

Closed sierra-moxon closed 3 years ago

sierra-moxon commented 3 years ago
codewarrior2000 commented 3 years ago

Perhaps it might be better to change in the README the wording from "This principal also applies ..." to "This principle also applies ..."

Thank you, Larry Chung Molecular Data Provider

CaseyTa commented 3 years ago

@sierra-moxon, could you please clarify where it says

"canonical translator predicates will not be tagged with the 'inverse:' attribute"

Does this mean that only the non-canonical predicate will specify which predicate is its inverse?

So take biolink:superclass_of and biolink:subclass_of. Currently, both have their inverses defined as each other. If biolink:subclass_of is defined as the canonical direction, then would it no longer specify inverse: superclass of, while biolink:superclass_of would keep its definition of inverse: subclass of? If this is the case, it might be slightly more inconvenient to find the inverse of a canonical predicate. It also seems unnecessary to remove the inverse if the canonical predicate will also be specified by biolink:canonical_predicate.

This is just a very minor quibble, and either way, I'll approve of the changes.

sierra-moxon commented 3 years ago

@sierra-moxon, could you please clarify where it says

"canonical translator predicates will not be tagged with the 'inverse:' attribute"

Does this mean that only the non-canonical predicate will specify which predicate is its inverse?

So take biolink:superclass_of and biolink:subclass_of. Currently, both have their inverses defined as each other. If biolink:subclass_of is defined as the canonical direction, then would it no longer specify inverse: superclass of, while biolink:superclass_of would keep its definition of inverse: subclass of? If this is the case, it might be slightly more inconvenient to find the inverse of a canonical predicate. It also seems unnecessary to remove the inverse if the canonical predicate will also be specified by biolink:canonical_predicate.

This is just a very minor quibble, and either way, I'll approve of the changes.

Hi Casey! Yep, your example is inline with the intention of this principle. @cmungall's original idea for declaring the inverse was to only curate the predicate in one direction (ie: only the non-canonical predicate would have the 'inverse' attribute populated). We added the biolink:canonical_predicate to make this a bit simpler to pull out from the canonical predicate itself. We did open a ticket in the BMT repo, to make a helper function for this as well: https://github.com/biolink/biolink-model-toolkit/issues/30.