Skip to content

Implemented removal of reagents of zero coefficient#93

Merged
JamesJeffryes merged 1 commit into
masterfrom
rebalancing_reactions
Jun 11, 2018
Merged

Implemented removal of reagents of zero coefficient#93
JamesJeffryes merged 1 commit into
masterfrom
rebalancing_reactions

Conversation

@samseaver

Copy link
Copy Markdown
Contributor

I tested this by artificially modifying a compound's formula.

@JamesJeffryes JamesJeffryes left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Big ask here is looking into merging the logic in Adjust_Reaction_Protons.py and Rebalance_Reactions.py

for rxn in sorted(Reactions_Dict.keys()):
# if(rxn != "rxn00013"):
# continue

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we remove this?


old_stoichiometry=Reactions_Dict[rxn]["stoichiometry"]
Rxn_Cpds_Array=ReactionsHelper.parseStoich(old_stoichiometry)
print "1: ",Rxn_Cpds_Array

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could be wordy. Is it needed?

Rxn_Cpds_Array=ReactionsHelper.parseStoich(old_stoichiometry)
print "1: ",Rxn_Cpds_Array
ReactionsHelper.adjustCompound(Rxn_Cpds_Array,"cpd00067",float(number))
print "2: ",Rxn_Cpds_Array

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could be wordy. Is it needed?

Update_Reactions=0
for rxn in sorted(Reactions_Dict.keys()):
# if(rxn != "rxn00013"):
# continue

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This code seems to duplicate logic in Adjust_Reaction_Protons.py. Can we consolidate it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No, it started off like that, but it got tricky. There is an internal self-reference between status and stoichiometry when attempting to add/remove compounds, and it got more complex when considering whether users would add new compounds with an incorrectly stated number of protons. It was easier for me to separate it out into two scripts.

@samseaver

Copy link
Copy Markdown
Contributor Author

Can you merge, I want to pull it into the other branch.

@JamesJeffryes JamesJeffryes merged commit ac69b93 into master Jun 11, 2018
@samseaver samseaver deleted the rebalancing_reactions branch June 11, 2018 18:38
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.

2 participants