opencobra / memote

memote – the genome-scale metabolic model test suite
https://memote.readthedocs.io/
Apache License 2.0
126 stars 26 forks source link

untagged_normal_transport identifies terminal oxidases & proton pumps as transporters #141

Closed ChristianLieven closed 6 years ago

ChristianLieven commented 7 years ago

Reactions like this should probably be excepted:

Na(+)-translocating NADH-quinone - NQR h_c + 2.0 na1_c + nadh_c + q8_im --> 2.0 na1_p + nad_c + q8h2_im

Cytochrome oxidase bd (ubiquinol-8: 2 protons) 2.0 h_c + 0.5 o2_c + q8h2_c ⇌ h2o_c + q8_c + 2.0 h_e

Nitrite reductase (cytochrome; ammonia-forming) 6 focytcc553_p + 8 h_c + no2_c --> 6 ficytcc553_p + 2 h2o_c + nh4_c

ChristianLieven commented 7 years ago

Technically this is the correct and desired behaviour of the function identifying transport reactions, as in each case protons are transported. Since this is really only a problem when considering that 'traditionally' the reactions of the respiratory chain have been exempt from bearing the 't' tag in the BiGG nomeclature, I think the implementation of such exceptions should be restricted to corresponding function in syntax.py rather than the basic.py function that identifies.

The only rule regarding the 't' tag that I know is the very broad:

screen shot 2017-08-26 at 10 25 59

From the supplement of Thiele, I., & Palsson, B. Ø. (2010, January). A protocol for generating a high-quality genome-scale metabolic reconstruction. Nature protocols. Nature Publishing Group. http://doi.org/10.1038/nprot.2009.203

ChristianLieven commented 7 years ago

So, I guess what I am trying to say is that I don't think this is a bug as long as there is an uncertainty as to the extend of the definition of "transport reactions" within the BiGG nomenklature.

@zakandrewking, how do you handle these suffixes in BiGG so far? What reactions would you consider as transport reactions and what would you think should be exceptions?

zakandrewking commented 7 years ago

We don't attempt to normalize transporter suffixes in BiGG. In this case, the cellular location suffixes already seem applicable to the reactions, so an extra label would be difficult to apply.

FYI, we are also trying to remove reversible/irreversible suffixes when we are changing IDs because BiGG identifiers are not direction-specific.

ChristianLieven commented 7 years ago

FYI, we are also trying to remove reversible/irreversible suffixes when we are changing IDs because BiGG identifiers are not direction-specific.

Thanks for the heads-up! I don't think thats something I am checking for with memote, so I won't implement that in the future.

What do you mean by:

In this case, the cellular location suffixes already seem applicable to the reactions, so an extra label would be difficult to apply.

I mean, in Python at least handling strings is fairly straightforward and to me it makes sense to use tags to be able to distinguish between 2 identical reactions in different compartments (FUM vs. FUMm) and a catalytic reaction and a homonymous transporter (FUM vs FUMt). Let me know in case you're redefining the above suffix table significantly.

I'll adapt memote accordingly.

zakandrewking commented 7 years ago

You can ignore that comment. I agree with your examples, and I think the existing standard works fine. We can always differentiate between slightly different reactions with a different ID (e.g. FUMt vs. FUM1t).

ChristianLieven commented 7 years ago

We can always differentiate between slightly different reactions with a different ID (e.g. FUMt vs. FUM1t).

Yeah good call! That sounds reasonable. As for 'electron-transporting'/'translocating' reactions, I will count them as transporters for now then, until anyone suggests a good way of distinguishing them from other transporters.

Midnighter commented 6 years ago

Syntax checks are currently disabled.