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

Recognizing SBO annotations using lowercase "sbo". #477

Closed
jonovik opened this issue Jul 27, 2018 · 4 comments
Closed

Recognizing SBO annotations using lowercase "sbo". #477

jonovik opened this issue Jul 27, 2018 · 4 comments
Assignees
Milestone

Comments

@jonovik
Copy link
Contributor

jonovik commented Jul 27, 2018

elem.annotation is None or 'SBO' not in elem.annotation]

I propose to replace "SBO" with "sbo" in the code above.

My SBO annotations are not recognized because I use lowercase "sbo" as the key, not "SBO". Lowercase sbo is the identifiers.org (Juty et al. 2012) namespace for SBO, and I like to use identifiers.org whenever possible. In particular I like the immediate reference to the ontology used; for example, http://identifiers.org/sbo explains what ontology http://identifiers.org/sbo/SBO:0000249 is from.

I think identifiers.org namespaces are a better alternative than hardcoding "SBO" into sbo.py, and the metabolite and reaction annotation checks all use identifiers.org namespace-identifiers: pubchem.compound kegg.compound seed.compound inchikey chebi hmdb reactome metanetx.chemical bigg.metabolite biocyc rhea kegg.reaction metanetx.reaction bigg.reaction ec-code brenda

@ChristianLieven

@jonovik
Copy link
Contributor Author

jonovik commented Jul 27, 2018

I also suggest similar changes in other files, replacing
["']SBO["']
with
'sbo'

The change would affect the following files:

$ ag ["']SBO["'] memote

memote/tests/test_for_support/test_for_helpers.py
224: rxn.annotation["SBO"] = "SBO:0000655"

memote/memote/support/helpers.py
165: 'SBO' in rxn.annotation and
166: rxn.annotation['SBO'] in TRANSPORT_RXN_SBO_TERMS])
306: 'SBO' in rxn.annotation and
307: rxn.annotation['SBO'] == 'SBO:0000629'])

memote/memote/support/sbo.py
45: elem.annotation is None or 'SBO' not in elem.annotation]
69: 'SBO' not in elem.annotation or
70: elem.annotation['SBO'] not in term]

memote/tests/test_for_support/test_for_helpers.py
390: rxn1.annotation = {'SBO': 'SBO:0000629'}

memote/tests/test_for_support/test_for_sbo.py
48: met.annotation = {'SBO': 'SBO:1', 'bigg.metabolite': 'dad_2'}

@ChristianLieven
Copy link
Contributor

Cheers @jonovik that was an oversight on our part. We want to facilitate MIRIAM-compliant behavior wherever possible.

Will be fixed in the next release!

@ChristianLieven
Copy link
Contributor

I should also point out that the current sbo.py will likely be reworked in the future:
#469

@kvikshaug kvikshaug added this to the Sprint 39 milestone Sep 27, 2018
@ChristianLieven
Copy link
Contributor

Fixed in #486. Please reopen this issue if there are any remaining problems.

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

No branches or pull requests

3 participants