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

Templates #56

Closed
wants to merge 6 commits into from
Closed

Templates #56

wants to merge 6 commits into from

Conversation

JamesJeffryes
Copy link
Contributor

This PR updates the templates by replacing reactions marked as obsolete in the template with a valid linked reaction. Also, reactions that were not atom balanced were removed from the template. (only 9 from core & gram +&- but many more from human & mycobacterial templates) The Validate Template file has been updated to check for obsolete and imbalanced reactions on travis.

I've requested review from those that might have an opinion on changes to the templates.

Fixes #48

Copy link
Contributor

@mmundy42 mmundy42 left a comment

Choose a reason for hiding this comment

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

For reactions that have a type of “gapfilling” it should be fine to remove them from the template. But I did notice that some of the reactions have a type of
“conditional” which means they are linked to a complex and a role. I would prefer to keep conditional reactions since they improve the reconstruction
of draft models.

@mmundy42
Copy link
Contributor

Yes, that is a good compromise. We can review later and figure out if the troublesome reactions can be replaced by reactions that have OK status.

Does this sound like a good compromise? I would change the validation to allow imbalanced reactions if they are conditional.

@samseaver
Copy link
Contributor

Unbalanced reactions should never be allowed in the template, but if there are conditional reactions that are unbalanced, they should either be fixed directly, or replaced with the appropriate balanced reaction.

Since these files have not yet been updated from the current objects being used in production, I have no problem removing them for the time being so that the checks pass, but we should maintain a separate file (added to this PR if possible) that lists: The reaction being removed and the template it was found in.

When time comes to actually maintain the templates through this repository, we'll have to curate these.

@janakagithub
Copy link
Contributor

James, do you save those unbalanced reactions from core/gram-/+ templates? I think some of those are key reactions (may be all the reactions), those need to be atom balanced and added back to the template.

@JamesJeffryes
Copy link
Contributor Author

Here are the reactions that are imbalanced. Some reactions like DNA synthesis cannot be balanced but we can change the status to OK if we want to keep them.
imbalanced rxns.txt

@JamesJeffryes
Copy link
Contributor Author

@janakagithub @jplfaria Do you want to attempt to balance these reactions?

@chian
Copy link

chian commented Aug 29, 2017 via email

…ore nuanced approach and updated the biochemistry to fix the reactions. The Mayo clinic templates are not checked
@JamesJeffryes
Copy link
Contributor Author

So I changed approach on this. The Mayo Clinic templates are exempt from validation while I was able to update or balance reactions for all but rxn01659. This reaction produces a plant hormone and is only used for gapfilling so I think it's reasonable to drop.

Copy link
Contributor

@samseaver samseaver left a comment

Choose a reason for hiding this comment

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

The change to formula in the Fatty-acyl-ACPs isn't right. I'd know because I tried to systematically change them all some time ago. We need to explicitly include the formula for the 4-Phosphopantetheine side chain because it's a key cofactor biosynthesized directly from a vitamin.

You'll find that all fatty-acyl acps, as far as I could, have this formula, and as such, if you have unbalanced reactions, then you need to change the formula of the fatty-acyl acp that is on the other side of the reaction (which is in fact common to both reactions here, so, evidently, I missed one)

@JamesJeffryes
Copy link
Contributor Author

So I'm going to close this PR and make the changes in the JSON files

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.

5 participants