diff --git a/cobra/core/group.py b/cobra/core/group.py index 5f65bd01e..079eada4a 100644 --- a/cobra/core/group.py +++ b/cobra/core/group.py @@ -1,48 +1,56 @@ # -*- coding: utf-8 -*- +"""Define the group class.""" + from __future__ import absolute_import -from cobra.core.object import Object -from six import string_types from warnings import warn -kind_types = ["collection", "classification", "partonomy"] +from six import string_types + +from cobra.core.object import Object + + +KIND_TYPES = ("collection", "classification", "partonomy") class Group(Object): - """Group is a class for holding information regarding - a pathways, subsystems, or other custom groupings of objects - within a cobra.Model object. + """ + Manage groups via this implementation of the SBML group specification. + + `Group` is a class for holding information regarding a pathways, subsystems, + or other custom groupings of objects within a cobra.Model object. Parameters ---------- - id : string + id : str The identifier to associate with this group - name : string + name : str, optional A human readable name for the group - members : list + members : iterable, optional A list object containing references to cobra.Model-associated objects that belong to the group. - kind : string - The kind of group, as specified for the Groups feature in the SBML - level 3 package specification. Can be any of "classification", - "partonomy", or "collection". Please consult the SBML level 3 package - specification to ensure you are using the proper value for kind. In - short, members of a "classification" group should have an "is-a" - relationship to the group (e.g. member is-a polar compound, or + kind : {"collection", "classification", "partonomy"}, optional + The kind of group, as specified for the Groups feature in the SBML level + 3 package specification. Can be any of "classification", "partonomy", or + "collection". The default is "collection". Please consult the SBML level + 3 package specification to ensure you are using the proper value for + kind. In short, members of a "classification" group should have an + "is-a" relationship to the group (e.g. member is-a polar compound, or member is-a transporter). Members of a "partonomy" group should have a - "part-of" relationship (e.g. member is part-of glycolysis). Members of - a "collection" group do not have an implied relationship between the + "part-of" relationship (e.g. member is part-of glycolysis). Members of a + "collection" group do not have an implied relationship between the members, so use this value for kind when in doubt (e.g. member is a gap-filled reaction, or member is involved in a disease phenotype). + """ - def __init__(self, id=None, name='', members=[], kind=''): + def __init__(self, id, name='', members=None, kind=None): Object.__init__(self, id, name) - self._members = set(members) - self._kind = kind - + self._members = set() if members is None else set(members) + self._kind = None + self.kind = "collection" if kind is None else kind # self.model is None or refers to the cobra.Model that # contains self self._model = None @@ -50,66 +58,51 @@ def __init__(self, id=None, name='', members=[], kind=''): # read-only @property def members(self): - return getattr(self, "_members", None) - - @members.setter - def members(self, members): - self._members = set(members) + return self._members @property def kind(self): - return getattr(self, "_kind", '') + return self._kind @kind.setter def kind(self, kind): - if kind in kind_types: + if kind in KIND_TYPES: self._kind = kind else: - raise ValueError("kind can only by one of: " + str(kind_types)) - - @property - def model(self): - """returns the model the group is a part of""" - return self._model + raise ValueError( + "Kind can only by one of: {}.".format(", ".join(KIND_TYPES))) - def add_members(self, members_list): - """Add objects to the group. + def add_members(self, new_members): + """ + Add objects to the group. Parameters ---------- - members_to_add : list - list of cobrapy objects to add to the group. + new_members : list + A list of cobrapy objects to add to the group. + """ - if isinstance(members_list, string_types) or \ - hasattr(members_list, "id"): + if isinstance(new_members, string_types) or \ + hasattr(new_members, "id"): warn("need to pass in a list") - members_list = [members_list] - - new_members = [] - _id_to_members = dict([(x.id, x) for x in self._members]) + new_members = [new_members] - # Check for duplicate members in the group - for member in members_list: - # we only need to add the member if it ins't already in the group - if member.id not in _id_to_members: - new_members.append(member) + self._members.update(new_members) - self._members = self._members.union(set(new_members)) - - def remove(self, members_list): - """Remove objects from the group. + def remove(self, to_remove): + """ + Remove objects from the group. Parameters ---------- - members_to_remove : list - list of cobrapy objects to remove from the group + to_remove : list + A list of cobrapy objects to remove from the group """ - if isinstance(members_list, string_types) or \ - hasattr(members_list, "id"): + if isinstance(to_remove, string_types) or \ + hasattr(to_remove, "id"): warn("need to pass in a list") - members_list = [members_list] + to_remove = [to_remove] - for member in members_list: - self._members.discard(member) + self._members.difference_update(to_remove) diff --git a/cobra/core/model.py b/cobra/core/model.py index 9adf0248e..64d9b652c 100644 --- a/cobra/core/model.py +++ b/cobra/core/model.py @@ -696,7 +696,7 @@ def add_groups(self, group_list): Parameters ---------- group_list : list - A list of `cobra.Group` objects to add to the model + A list of `cobra.Group` objects to add to the model. """ def existing_filter(group): @@ -723,21 +723,24 @@ def existing_filter(group): if isinstance(member, Reaction): if member not in self.reactions: self.add_reactions([member]) - if isinstance(member, Gene): - if member not in self.genes: - self.add_genes([member]) + # TODO(midnighter): `add_genes` method does not exist. + # if isinstance(member, Gene): + # if member not in self.genes: + # self.add_genes([member]) self.groups += [group] def remove_groups(self, group_list): - """Remove groups from the model. Members of each group are not removed + """Remove groups from the model. + + Members of each group are not removed from the model (i.e. metabolites, reactions, and genes in the group stay in the model after any groups containing them are removed). Parameters ---------- group_list : list - A list of `cobra.Group` objects to remove from the model + A list of `cobra.Group` objects to remove from the model. """ if isinstance(group_list, string_types) or \ @@ -747,11 +750,8 @@ def remove_groups(self, group_list): for group in group_list: # make sure the group is in the model - try: - group = self.groups[self.groups.index(group)] - except ValueError: - warn('%s not in %s' % (group, self)) - # if there was no exception, remove the group + if group.id not in self.groups: + LOGGER.warning("%r not in %r. Ignored.", group, self) else: self.groups.remove(group) group._model = None diff --git a/cobra/test/test_model.py b/cobra/test/test_model.py index 450d167ff..9460374a5 100644 --- a/cobra/test/test_model.py +++ b/cobra/test/test_model.py @@ -337,7 +337,7 @@ def test_group_add_elements(self, model): num_members = 5 reactions_for_group = model.reactions[0:num_members] group = Group("arbitrary_group1") - group.members = reactions_for_group + group.add_members(reactions_for_group) group.kind = "collection" # number of member sin group should equal the number of reactions # assigned to the group @@ -353,7 +353,7 @@ def test_group_kind(self): group = Group("arbitrary_group1") with pytest.raises(ValueError) as excinfo: group.kind = "non-SBML compliant group kind" - assert "kind can only by one of:" in str(excinfo.value) + assert "Kind can only by one of:" in str(excinfo.value) group.kind = "collection" assert group.kind == "collection" @@ -634,11 +634,11 @@ def test_group_model_reaction_association(self, model): num_members = 5 reactions_for_group = model.reactions[0:num_members] group = Group("arbitrary_group1") - group.members = reactions_for_group + group.add_members(reactions_for_group) group.kind = "collection" model.add_groups([group]) # group should point to and be associated with the model - assert group.model == model + assert group._model is model assert group in model.groups # model.get_associated_groups should find the group for each reaction @@ -650,7 +650,7 @@ def test_group_model_reaction_association(self, model): # longer associated with the group model.remove_groups([group]) assert group not in model.groups - assert group.model is not model + assert group._model is not model for reaction in reactions_for_group: assert group not in model.get_associated_groups(reaction) @@ -660,7 +660,7 @@ def test_group_members_add_to_model(self, model): reactions_for_group = model.reactions[0:num_members] model.remove_reactions(reactions_for_group, remove_orphans=False) group = Group("arbitrary_group1") - group.members = reactions_for_group + group.add_members(reactions_for_group) group.kind = "collection" # the old reactions should not be in the model for reaction in reactions_for_group: @@ -680,7 +680,7 @@ def test_group_loss_of_elements(self, model): elements_for_group.extend(model.metabolites[0:num_members_each]) elements_for_group.extend(model.genes[0:num_members_each]) group = Group("arbitrary_group1") - group.members = elements_for_group + group.add_members(elements_for_group) group.kind = "collection" model.add_groups([group])