Skip to content

Commit 2558642

Browse files
committed
refactor: adjust interface and tests
1 parent 13d683a commit 2558642

File tree

3 files changed

+72
-79
lines changed

3 files changed

+72
-79
lines changed

cobra/core/group.py

+54-61
Original file line numberDiff line numberDiff line change
@@ -1,115 +1,108 @@
11
# -*- coding: utf-8 -*-
22

3+
"""Define the group class."""
4+
35
from __future__ import absolute_import
46

5-
from cobra.core.object import Object
6-
from six import string_types
77
from warnings import warn
88

9-
kind_types = ["collection", "classification", "partonomy"]
9+
from six import string_types
10+
11+
from cobra.core.object import Object
12+
13+
14+
KIND_TYPES = ("collection", "classification", "partonomy")
1015

1116

1217
class Group(Object):
13-
"""Group is a class for holding information regarding
14-
a pathways, subsystems, or other custom groupings of objects
15-
within a cobra.Model object.
18+
"""
19+
Manage groups via this implementation of the SBML group specification.
20+
21+
`Group` is a class for holding information regarding a pathways, subsystems,
22+
or other custom groupings of objects within a cobra.Model object.
1623
1724
Parameters
1825
----------
19-
id : string
26+
id : str
2027
The identifier to associate with this group
21-
name : string
28+
name : str, optional
2229
A human readable name for the group
23-
members : list
30+
members : iterable, optional
2431
A list object containing references to cobra.Model-associated objects
2532
that belong to the group.
26-
kind : string
27-
The kind of group, as specified for the Groups feature in the SBML
28-
level 3 package specification. Can be any of "classification",
29-
"partonomy", or "collection". Please consult the SBML level 3 package
30-
specification to ensure you are using the proper value for kind. In
31-
short, members of a "classification" group should have an "is-a"
32-
relationship to the group (e.g. member is-a polar compound, or
33+
kind : {"collection", "classification", "partonomy"}, optional
34+
The kind of group, as specified for the Groups feature in the SBML level
35+
3 package specification. Can be any of "classification", "partonomy", or
36+
"collection". The default is "collection". Please consult the SBML level
37+
3 package specification to ensure you are using the proper value for
38+
kind. In short, members of a "classification" group should have an
39+
"is-a" relationship to the group (e.g. member is-a polar compound, or
3340
member is-a transporter). Members of a "partonomy" group should have a
34-
"part-of" relationship (e.g. member is part-of glycolysis). Members of
35-
a "collection" group do not have an implied relationship between the
41+
"part-of" relationship (e.g. member is part-of glycolysis). Members of a
42+
"collection" group do not have an implied relationship between the
3643
members, so use this value for kind when in doubt (e.g. member is a
3744
gap-filled reaction, or member is involved in a disease phenotype).
45+
3846
"""
3947

40-
def __init__(self, id=None, name='', members=[], kind=''):
48+
def __init__(self, id, name='', members=None, kind=None):
4149
Object.__init__(self, id, name)
4250

43-
self._members = set(members)
44-
self._kind = kind
45-
51+
self._members = set() if members is None else set(members)
52+
self._kind = None
53+
self.kind = "collection" if kind is None else kind
4654
# self.model is None or refers to the cobra.Model that
4755
# contains self
4856
self._model = None
4957

5058
# read-only
5159
@property
5260
def members(self):
53-
return getattr(self, "_members", None)
54-
55-
@members.setter
56-
def members(self, members):
57-
self._members = set(members)
61+
return self._members
5862

5963
@property
6064
def kind(self):
61-
return getattr(self, "_kind", '')
65+
return self._kind
6266

6367
@kind.setter
6468
def kind(self, kind):
65-
if kind in kind_types:
69+
if kind in KIND_TYPES:
6670
self._kind = kind
6771
else:
68-
raise ValueError("kind can only by one of: " + str(kind_types))
69-
70-
@property
71-
def model(self):
72-
"""returns the model the group is a part of"""
73-
return self._model
72+
raise ValueError(
73+
"Kind can only by one of: {}.".format(", ".join(KIND_TYPES)))
7474

75-
def add_members(self, members_list):
76-
"""Add objects to the group.
75+
def add_members(self, new_members):
76+
"""
77+
Add objects to the group.
7778
7879
Parameters
7980
----------
80-
members_to_add : list
81-
list of cobrapy objects to add to the group.
81+
new_members : list
82+
A list of cobrapy objects to add to the group.
83+
8284
"""
8385

84-
if isinstance(members_list, string_types) or \
85-
hasattr(members_list, "id"):
86+
if isinstance(new_members, string_types) or \
87+
hasattr(new_members, "id"):
8688
warn("need to pass in a list")
87-
members_list = [members_list]
88-
89-
new_members = []
90-
_id_to_members = dict([(x.id, x) for x in self._members])
89+
new_members = [new_members]
9190

92-
# Check for duplicate members in the group
93-
for member in members_list:
94-
# we only need to add the member if it ins't already in the group
95-
if member.id not in _id_to_members:
96-
new_members.append(member)
91+
self._members.update(new_members)
9792

98-
self._members = self._members.union(set(new_members))
99-
100-
def remove(self, members_list):
101-
"""Remove objects from the group.
93+
def remove(self, to_remove):
94+
"""
95+
Remove objects from the group.
10296
10397
Parameters
10498
----------
105-
members_to_remove : list
106-
list of cobrapy objects to remove from the group
99+
to_remove : list
100+
A list of cobrapy objects to remove from the group
107101
"""
108102

109-
if isinstance(members_list, string_types) or \
110-
hasattr(members_list, "id"):
103+
if isinstance(to_remove, string_types) or \
104+
hasattr(to_remove, "id"):
111105
warn("need to pass in a list")
112-
members_list = [members_list]
106+
to_remove = [to_remove]
113107

114-
for member in members_list:
115-
self._members.discard(member)
108+
self._members.difference_update(to_remove)

cobra/core/model.py

+11-11
Original file line numberDiff line numberDiff line change
@@ -696,7 +696,7 @@ def add_groups(self, group_list):
696696
Parameters
697697
----------
698698
group_list : list
699-
A list of `cobra.Group` objects to add to the model
699+
A list of `cobra.Group` objects to add to the model.
700700
"""
701701

702702
def existing_filter(group):
@@ -723,21 +723,24 @@ def existing_filter(group):
723723
if isinstance(member, Reaction):
724724
if member not in self.reactions:
725725
self.add_reactions([member])
726-
if isinstance(member, Gene):
727-
if member not in self.genes:
728-
self.add_genes([member])
726+
# TODO(midnighter): `add_genes` method does not exist.
727+
# if isinstance(member, Gene):
728+
# if member not in self.genes:
729+
# self.add_genes([member])
729730

730731
self.groups += [group]
731732

732733
def remove_groups(self, group_list):
733-
"""Remove groups from the model. Members of each group are not removed
734+
"""Remove groups from the model.
735+
736+
Members of each group are not removed
734737
from the model (i.e. metabolites, reactions, and genes in the group
735738
stay in the model after any groups containing them are removed).
736739
737740
Parameters
738741
----------
739742
group_list : list
740-
A list of `cobra.Group` objects to remove from the model
743+
A list of `cobra.Group` objects to remove from the model.
741744
"""
742745

743746
if isinstance(group_list, string_types) or \
@@ -747,11 +750,8 @@ def remove_groups(self, group_list):
747750

748751
for group in group_list:
749752
# make sure the group is in the model
750-
try:
751-
group = self.groups[self.groups.index(group)]
752-
except ValueError:
753-
warn('%s not in %s' % (group, self))
754-
# if there was no exception, remove the group
753+
if group.id not in self.groups:
754+
LOGGER.warning("%r not in %r. Ignored.", group, self)
755755
else:
756756
self.groups.remove(group)
757757
group._model = None

cobra/test/test_model.py

+7-7
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,7 @@ def test_group_add_elements(self, model):
337337
num_members = 5
338338
reactions_for_group = model.reactions[0:num_members]
339339
group = Group("arbitrary_group1")
340-
group.members = reactions_for_group
340+
group.add_members(reactions_for_group)
341341
group.kind = "collection"
342342
# number of member sin group should equal the number of reactions
343343
# assigned to the group
@@ -353,7 +353,7 @@ def test_group_kind(self):
353353
group = Group("arbitrary_group1")
354354
with pytest.raises(ValueError) as excinfo:
355355
group.kind = "non-SBML compliant group kind"
356-
assert "kind can only by one of:" in str(excinfo.value)
356+
assert "Kind can only by one of:" in str(excinfo.value)
357357

358358
group.kind = "collection"
359359
assert group.kind == "collection"
@@ -634,11 +634,11 @@ def test_group_model_reaction_association(self, model):
634634
num_members = 5
635635
reactions_for_group = model.reactions[0:num_members]
636636
group = Group("arbitrary_group1")
637-
group.members = reactions_for_group
637+
group.add_members(reactions_for_group)
638638
group.kind = "collection"
639639
model.add_groups([group])
640640
# group should point to and be associated with the model
641-
assert group.model == model
641+
assert group._model is model
642642
assert group in model.groups
643643

644644
# model.get_associated_groups should find the group for each reaction
@@ -650,7 +650,7 @@ def test_group_model_reaction_association(self, model):
650650
# longer associated with the group
651651
model.remove_groups([group])
652652
assert group not in model.groups
653-
assert group.model is not model
653+
assert group._model is not model
654654
for reaction in reactions_for_group:
655655
assert group not in model.get_associated_groups(reaction)
656656

@@ -660,7 +660,7 @@ def test_group_members_add_to_model(self, model):
660660
reactions_for_group = model.reactions[0:num_members]
661661
model.remove_reactions(reactions_for_group, remove_orphans=False)
662662
group = Group("arbitrary_group1")
663-
group.members = reactions_for_group
663+
group.add_members(reactions_for_group)
664664
group.kind = "collection"
665665
# the old reactions should not be in the model
666666
for reaction in reactions_for_group:
@@ -680,7 +680,7 @@ def test_group_loss_of_elements(self, model):
680680
elements_for_group.extend(model.metabolites[0:num_members_each])
681681
elements_for_group.extend(model.genes[0:num_members_each])
682682
group = Group("arbitrary_group1")
683-
group.members = elements_for_group
683+
group.add_members(elements_for_group)
684684
group.kind = "collection"
685685
model.add_groups([group])
686686

0 commit comments

Comments
 (0)