Refactor cobra.Model and cobra.Reaction

[pstjohn]

This is something I'd like to get to eventually. Hopefully we could keep these classes relatively clean and put a lot of the implementation in other files.

Some candidates to move:

• Reaction.build_reaction_string, Reaction.build_reaction_from_string
  These could easily go into a cobra/utils/reaction_string.py or something similar.

• Model.copy and Reaction.copy, could go into a utils/copy.py

• Model.add_reactions and similar
  These we could put in a cobra/utils/combine.py, and do something like def add_reaction_to_model, add_reaction_to_reaction, etc. If we thought it was worth it, it could then be pretty trivial to add in a bunch more functionality to __add__, checking the type of the other class and calling the correct routine as appropriate.

• all the optlang stuff 😄 . When/where it makes sense at least

  • upper_bound and lower_bound

Note: From cobrapy meeting, lets prioritize where methods can be reused.

[cdiener]

I would add:

• refactor or deprecate Reaction.objective_coefficient
• move new optlang solver configs and helpers into cobra.solvers

[phantomas1234]

I think we need strike a balance between bloated core classes and scattered code. I see Model.add_reactions as one of the most important methods of Model. Wouldn't it be extremely inconvenient for the user to have to import basic functionality like this from some util module? I admit that we've been going a little bit too far with bloating the cameo core classes, but we're not planning to move all of it into cobrapy.

[pstjohn]

so I've been thinking more about context managers and adding together models / reactions / metabolites...

It would be great if we could add reactions to a model in a context, and have them removed after the model leaves that context. (Same with metabolites, adding metabolites to reactions, etc.)

If this is the behavior we want, we're going to need sets of functions that are exact inverses of eachother: i.e., model.add_reaction, reaction.remove_from_model(), etc.

Rather than keeping these scattered across the various classes, I think we should these in a centralized file (cobra.manipulation.addition) or similar.

We could define a bunch of methods,
add_reactions_to_model, add_metabolites_to_model, and their inverses subtract_reactions_from_model, etc.

In Object.py, we could define __iadd__, __isub__ methods that check the two classes being added and call the relevant add or remove function. Then it would be super easy to context-manage all these modifications: we just check for context and queue up the inverse method later.

We'll keep the existing functions around for compatibility, but we may want to standardize them a bit. (Why, for instance, does Model.add_reactions yell at you when you pass a single reaction, but Model.add_metabolites just converts to a DictList for you?)

[hredestig]

To continue this discussion how about we for:

reactions

• resolve the add_reaction/add_reactions to single function
• merge reaction.remove_from_model and reaction.delete to reduce code-copying
• reaction.remove_from_model and model.add_reactions respect context

metabolites

• add remove_metabolites to cobra.model to enable reversing add_metabolites

genes

• add the gene.knock_out from cameo. Seems to me that
  cobra.manipulation.deletion.{remove_genes, delete_model_genes} are
  not necessary to be supported reversibly(?)
• refactor gene.remove_from_model to use the suggested functions

[pstjohn]

We can probably close this for now, thoughts?
A lot of the previous comments got addressed in #404 (except for the genes), but that might want to be a separate issue

[hredestig]

Yup agree, whatever is left here should better be made to a more concrete issue, so let's close.