Skip to content

Commit

Permalink
Fixed a bug in ArrayBasedModel bounds setting in Python 3 (#285)
Browse files Browse the repository at this point in the history
* Fixed a bug in ArrayBasedModel bounds setting in Python 3.

See https://groups.google.com/forum/#!topic/cobra-pie/-Yfx7atjII8

Python 3 calls only __setitem__ and not __setslice__ as Python 2
and there was an error in the LinkedArray class in __setitem__
where it would assign all the values to each attribute in place
of assigning each value to the correct attribute.

Also added a unit test so that behavior is checked in the future...

* Refactoring and fixed another bug.

The __setslice__ did not handle the l[i:j] = [...] case correctly.
It would try to access elements i to j in [...] as well and not
0 to j-i as it should be.
  • Loading branch information
cdiener authored and hredestig committed Oct 7, 2016
1 parent 643ebf7 commit c05bad8
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 12 deletions.
21 changes: 10 additions & 11 deletions cobra/core/ArrayBasedModel.py
Original file line number Diff line number Diff line change
Expand Up @@ -320,21 +320,20 @@ def __init__(self, list, attribute):
def __setitem__(self, index, value):
ndarray.__setitem__(self, index, value)
if isinstance(index, slice):
for i, entry in enumerate(self._list[index]):
setattr(entry, self._attr, value)
# not sure why that is here
if index.stop == maxsize:
index = slice(index.start, len(self))
if hasattr(value, "__getitem__"):
for i, entry in enumerate(self._list[index]):
setattr(entry, self._attr, value[i])
else:
for i, entry in enumerate(self._list[index]):
setattr(entry, self._attr, value)
else:
setattr(self._list[index], self._attr, value)

def __setslice__(self, i, j, value):
ndarray.__setitem__(self, slice(i, j), value)
if j == maxsize:
j = len(self)
if hasattr(value, "__getitem__"): # setting to a list
for index in range(i, j):
setattr(self._list[index], self._attr, value[index])
else:
for index in range(i, j):
setattr(self._list[index], self._attr, value)
self.__setitem__(slice(i, j), value)

def _extend(self, other):
old_size = len(self)
Expand Down
16 changes: 15 additions & 1 deletion cobra/test/unit_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,6 @@ def testGPR_modification(self):
model = self.model
reaction = model.reactions.get_by_id("PGI")
old_gene = list(reaction.genes)[0]
old_gene_reaction_rule = reaction.gene_reaction_rule
new_gene = model.genes.get_by_id("s0001")
# add an existing 'gene' to the gpr
reaction.gene_reaction_rule = 's0001'
Expand Down Expand Up @@ -779,6 +778,21 @@ def test_array_based_select(self):
with self.assertRaises(TypeError):
model.reactions[[True, False]]

def test_array_based_bounds_setting(self):
model = self.model
bounds = [0.0] * len(model.reactions)
model.lower_bounds = bounds
self.assertEqual(type(model.reactions[0].lower_bound), float)
self.assertAlmostEqual(model.reactions[0].lower_bound, 0.0)
model.upper_bounds[1] = 1234.0
self.assertAlmostEqual(model.reactions[1].upper_bound, 1234.0)
model.upper_bounds[9:11] = [100.0, 200.0]
self.assertAlmostEqual(model.reactions[9].upper_bound, 100.0)
self.assertAlmostEqual(model.reactions[10].upper_bound, 200.0)
model.upper_bounds[9:11] = 123.0
self.assertAlmostEqual(model.reactions[9].upper_bound, 123.0)
self.assertAlmostEqual(model.reactions[10].upper_bound, 123.0)


# make a test suite to run all of the tests
loader = TestLoader()
Expand Down

0 comments on commit c05bad8

Please sign in to comment.