Skip to content

Commit

Permalink
fixed according to comments
Browse files Browse the repository at this point in the history
  • Loading branch information
uri.akavia committed Aug 11, 2022
1 parent 7ea50ad commit 63ce057
Show file tree
Hide file tree
Showing 5 changed files with 128 additions and 75 deletions.
26 changes: 24 additions & 2 deletions src/cobra/core/gene.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
from ast import parse as ast_parse
from copy import deepcopy
from keyword import kwlist
from typing import FrozenSet, Iterable, Optional, Set, Tuple, Union
from typing import TYPE_CHECKING, FrozenSet, Iterable, Optional, Set, Tuple, Union
from warnings import warn

import sympy.logic.boolalg as spl
Expand All @@ -30,6 +30,10 @@
from cobra.util.util import format_long_string


if TYPE_CHECKING:
from cobra.core import Reaction


# TODO - When https://github.com/symengine/symengine.py/issues/334 is resolved,
# change sympy.Symbol (above in imports) to optlang.symbolics.Symbol

Expand Down Expand Up @@ -246,10 +250,28 @@ def knock_out(self) -> None:
context.
"""
self.functional = False
for reaction in self.reactions:
for reaction in self.reactions: # type: ignore
if not reaction.functional:
reaction.bounds = (0, 0)

@property
def reactions(self) -> FrozenSet["Reaction"]:
"""Return a frozenset of reactions.
If the gene is present in a model, calculates this set by querying
the model reactions for this gene.
If not, returns the _reaction field, see cobra.Species.
Returns
-------
FrozenSet
A frozenset that includes the reactions of the gene.
"""
if self.model:
return frozenset(self.model.reactions.query(lambda x: self in x, "genes"))
else:
return frozenset(self._reaction)

def _repr_html_(self):
return f"""
<table>
Expand Down
24 changes: 22 additions & 2 deletions src/cobra/core/metabolite.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
"""Define the Metabolite class."""

import re
from typing import TYPE_CHECKING, Dict, Optional, Union
from typing import TYPE_CHECKING, Dict, FrozenSet, Optional, Union
from warnings import warn

from ..exceptions import OptimizationError
Expand All @@ -15,7 +15,7 @@
from optlang.interface import Container
from pandas import DataFrame

from cobra.core import Solution
from cobra.core import Reaction, Solution
from cobra.summary.metabolite_summary import MetaboliteSummary

# Numbers are not required because of the |(?=[A-Z])? block. See the
Expand Down Expand Up @@ -276,6 +276,26 @@ def remove_from_model(self, destructive: bool = False) -> None:
"""
self._model.remove_metabolites(self, destructive)

@property
def reactions(self) -> FrozenSet["Reaction"]:
"""Return a frozenset of reactions.
If the metabolite is present in a model, calculates this set by querying
the model reactions for this metabolite.
If not, returns the _reaction field, see cobra.Species.
Returns
-------
FrozenSet
A frozenset that includes the reactions of the metabolite.
"""
if self.model:
return frozenset(
self.model.reactions.query(lambda x: self in x, "metabolites")
)
else:
return frozenset(self._reaction)

def summary(
self,
solution: Optional["Solution"] = None,
Expand Down
59 changes: 22 additions & 37 deletions src/cobra/core/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -723,34 +723,24 @@ def existing_filter(rxn: Reaction) -> bool:
return True

# First check whether the reactions exist in the model. Copy reactions that
# belong to another model, in order not to remove them (see
# https://github.com/opencobra/cobrapy/issues/673#issuecomment-371361476).
exist_but_different_stoich = set()
# belong to another model, in order not to remove them from the original model.
exist_but_different = set()
other_model = ""
for reaction in filter(lambda x: not existing_filter(x), reaction_list):
met_id_dict = {
met.id: stoich for met, stoich in reaction.metabolites.items()
}
existing_met_id_dict = {
met.id: stoich
for met, stoich in self.reactions.get_by_id(
reaction.id
).metabolites.items()
}
if met_id_dict != existing_met_id_dict:
exist_but_different_stoich.add(reaction.id)
other_model = reaction.model
if len(exist_but_different_stoich):
if reaction != self.reactions.get_by_id(reaction.id):
exist_but_different.add(reaction.id)
other_model = other_model or reaction.model
if len(exist_but_different):
logger.warning(
f"The reactions {', '.join(exist_but_different_stoich)} exist"
f"The reactions {', '.join(exist_but_different)} exist"
f" in both {self} and {other_model}, but have different "
f"stoichiometries in the two mdoels. Please check to see "
f"which stoichiometry you'd like to add."
f"properties in the two mdoels. Please check to see "
f"which properties are the correct ones you'd like to add."
)
pruned = DictList(
[
reaction
if not reaction._model or reaction._model == self
if not reaction.model or reaction.model == self
else reaction.copy()
for reaction in filter(existing_filter, reaction_list)
]
Expand All @@ -763,20 +753,15 @@ def existing_filter(rxn: Reaction) -> bool:
reaction._model = self
if context:
context(partial(setattr, reaction, "_model", None))
met_id_dict = {
met.id: stoich for met, stoich in reaction.metabolites.items()
}
met_dict = {met.id: met for met in reaction.metabolites.keys()}
mets_to_add = [
met_dict[met_id]
for met_id in set(met_id_dict.keys()).difference(
self.metabolites.list_attr("id")
)
met
for met in reaction.metabolites
if met.id not in self.metabolites.list_attr("id")
]
self.add_metabolites(mets_to_add)
new_stoich = {
self.metabolites.get_by_id(met_id): stoich
for met_id, stoich in met_id_dict.items()
self.metabolites.get_by_id(met.id): stoich
for met, stoich in reaction.metabolites.items()
}
reaction.metabolites = new_stoich
reaction.update_genes_from_gpr()
Expand Down Expand Up @@ -846,15 +831,15 @@ def remove_reactions(
self.reactions.remove(reaction)
reaction._model = None

for met in reaction.metabolites:
for met in reaction._metabolites:
if reaction in met._reaction:
met.reaction_remove(reaction, context)
met.remove_reaction(reaction)
if remove_orphans and len(met.reactions) == 0:
self.remove_metabolites(met)

for gene in reaction.genes:
for gene in reaction._genes:
if reaction in gene._reaction:
gene.reaction_remove(reaction, context)
gene.remove_reaction(reaction)

if remove_orphans and len(gene.reactions) == 0:
self.genes.remove(gene)
Expand Down Expand Up @@ -1287,13 +1272,13 @@ def repair(
self.groups._generate_index()
if rebuild_relationships:
for met in self.metabolites:
met.reaction_clear()
met.clear_reaction()
for gene in self.genes:
gene.reaction_clear()
gene.clear_reaction()
for rxn in self.reactions:
rxn.update_genes_from_gpr()
for met in rxn.metabolites:
met.reaction_add(rxn)
met.add_reaction(rxn)

# point _model to self
for dict_list in (self.reactions, self.genes, self.metabolites, self.groups):
Expand Down
64 changes: 55 additions & 9 deletions src/cobra/core/reaction.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""Define the Reaction class."""

import hashlib
import math
import re
from collections import defaultdict
from copy import copy, deepcopy
Expand Down Expand Up @@ -594,7 +595,7 @@ def metabolites(self, value: Dict[Metabolite, float]) -> None:
self._metabolites = value
context = get_context(self)
for met in value.keys():
met.reaction_add(self, context)
met.add_reaction(self)

@property
def genes(self) -> FrozenSet:
Expand Down Expand Up @@ -858,9 +859,9 @@ def _update_awareness(self) -> None:
This function is deprecated and shouldn't be used.
"""
for x in self._metabolites:
x.reaction_add(self)
x.add_reaction(self)
for x in self._genes:
x.reaction_add(self)
x.add_reaction(self)

def remove_from_model(self, remove_orphans: bool = False) -> None:
"""Remove the reaction from a model.
Expand Down Expand Up @@ -951,7 +952,7 @@ def __setstate__(self, state: Dict) -> None:
self.__dict__.update(state)
for x in set(state["_metabolites"]).union(state["_genes"]):
x._model = self._model
x.reaction_add(self)
x.add_reaction(self)

def copy(self) -> "Reaction":
"""Copy a reaction.
Expand Down Expand Up @@ -1001,7 +1002,7 @@ def __add__(self, other: "Reaction") -> "Reaction":
"""
new_reaction = self.copy()
if other == 0:
if isinstance(other, int) and other == 0:
return new_reaction
else:
new_reaction += other
Expand Down Expand Up @@ -1285,7 +1286,7 @@ def add_metabolites(
self._metabolites[metabolite] = coefficient
# make the metabolite aware that it is involved in this
# reaction
metabolite.reaction_add(self)
metabolite.add_reaction(self)

# from cameo ...
model = self.model
Expand All @@ -1304,7 +1305,7 @@ def add_metabolites(
if the_coefficient == 0:
# make the metabolite aware that it no longer participates
# in this reaction
metabolite.reaction_remove(self)
metabolite.remove_reaction(self)
self._metabolites.pop(metabolite)

context = get_context(self)
Expand Down Expand Up @@ -1504,7 +1505,7 @@ def _associate_gene(self, cobra_gene: Gene) -> None:
"""
self._genes.add(cobra_gene)
cobra_gene.reaction_add(self)
cobra_gene.add_reaction(self)
cobra_gene._model = self._model

def _dissociate_gene(self, cobra_gene: Gene) -> None:
Expand All @@ -1516,7 +1517,7 @@ def _dissociate_gene(self, cobra_gene: Gene) -> None:
"""
self._genes.discard(cobra_gene)
cobra_gene.reaction_remove(self)
cobra_gene.remove_reaction(self)

def knock_out(self) -> None:
"""Knockout reaction by setting its bounds to zero."""
Expand Down Expand Up @@ -1673,6 +1674,51 @@ def summary(
fva=fva,
)

def __eq__(self, other) -> bool:
self_state = self.__getstate__()
self_state["_metabolites"] = {
met.id: stoic for met, stoic in self.metabolites.items()
}
self_state["_genes"] = {gene.id for gene in self.genes}
self_state.pop("_model")
other_state = other.__getstate__()
other_state["_metabolites"] = {
met.id: stoic for met, stoic in other.metabolites.items()
}
other_state["_genes"] = {gene.id for gene in other.genes}
other_state.pop("_model")

self_state.pop("_lower_bound")
self_state.pop("_upper_bound")
other_state.pop("_lower_bound")
other_state.pop("_upper_bound")

_tolerance = 1e-7
if self.model:
_tolerance = self.model.tolerance
elif other.model:
_tolerance = other.model.tolerance
elif self.model and other.model:
_tolerance = min(self.model.tolerance, other.model.tolerance)
if not math.isclose(self.lower_bound, other.lower_bound, abs_tol=_tolerance):
return False
elif not math.isclose(self.upper_bound, other.upper_bound, abs_tol=_tolerance):
return False
else:
return self_state == other_state

def __hash__(self) -> int:
"""Define __hash__ explicitly to allow usage of reaction in sets.
Python objects that define __eq__ MUST define __hash__ explicitly, or they
are unhashable. __hash__ is set to the __hash__ function of cobra.Object.
Returns
-------
int
"""
return Object.__hash__(self)

def __str__(self) -> str:
"""Return reaction id and reaction as str.
Expand Down
Loading

0 comments on commit 63ce057

Please sign in to comment.