Conversation
cdiener
left a comment
cdiener
left a comment
There was a problem hiding this comment.
Looks mostly good to me.
The necessity to add so many sync functions just to ensure Species._reactions mirrors what is defined in Model makes it really clear that it would be much easier with a "single source of truth" design where Species.reactions only looks through reactions in the models for whether they contain the species and returns an error if the Species in not part of a Model (where that function does not make much sense since that is a 1:1 mapping anyway for reactions that are not part of a model).
src/cobra/core/model.py
Outdated
| reaction._model = None | ||
|
|
||
| for met in reaction._metabolites: | ||
| for met in reaction.metabolites: |
There was a problem hiding this comment.
This is slower though since it creates a copy of the metabolite list. It's not great to access hidden attributes bu here it creates a large memory overhead not to do that.
There was a problem hiding this comment.
Should I change all other places where I access reaction.metabolites? I don't think so, but I'm not sure
added species.py functions to set ._reaction field made metabolites a settable property, and used that to simplify bits of model (including copy and add_reaction)
|
Fixed almost everything. If I remove context from species/remove_reaction, species/add_reaction almost everything works, which is simpler. However, test_remove_genes_with_context() breaks. Can you help me fix that? I'm not good with contexts. The previous version would not try to update contexts when calling add_reaction or remove_reaction from _associate_gene/__dissociate_gene and that would work. |
|
@cdiener - can you help me fix test_remove_genes_with_context() please |
2 participants
Fix Adding reactions from another model does not reset the reactions their metabolites refer to #673 by making reactions in species search the model fields, if this species is in a model.
Added checks in the code to copy the reaction if it is in a model that isn't the one it is added to.
Added properties reaction.metabolites as a settable property, added reaction_add, reaction_remove, reaction_clear to species.py, in order to minimize use of ._reaction.
test_add_reactions_with_context
test_add_reactions_from_another_model