Skip to content

Commit

Permalink
Ensure length check on unconstrained list always passes (#1172)
Browse files Browse the repository at this point in the history
* Ensure length check on unconstrained list always passes

* Add changelog entry

* Same fix for __delitem__
  • Loading branch information
mdickinson authored Jun 2, 2020
1 parent b663e2d commit 4dca1f2
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 4 deletions.
2 changes: 1 addition & 1 deletion CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ Features
* Add ``Map`` and ``PrefixMap`` trait types. (#886, #953, #956, #970, #1139)
* Add ``TraitList`` as the base list object that can perform validation
and emit change notifications. (#912, #981, #984, #989, #999, #1003, #1011,
#1026, #1009, #1040, #1173)
#1026, #1009, #1040, #1172, #1173)
* Add ``TraitDict`` as the base dict object that can perform validation and
emit change notifications. (#913)
* Add ``TraitSet`` as the base set object that can perform validation and
Expand Down
19 changes: 19 additions & 0 deletions traits/tests/test_trait_list_object.py
Original file line number Diff line number Diff line change
Expand Up @@ -1150,6 +1150,8 @@ class HasLengthConstrainedLists(HasTraits):

at_most_five = List(Int, maxlen=5)

unconstrained = List(Int)


class TestTraitListObject(unittest.TestCase):
def test_list_of_lists_pickle_with_notifier(self):
Expand Down Expand Up @@ -1214,6 +1216,11 @@ def test_delitem_slice_too_small(self):
del foo.at_least_two[:]
self.assertEqual(foo.at_least_two, [1, 2])

def test_delitem_from_empty(self):
foo = HasLengthConstrainedLists()
with self.assertRaises(IndexError):
del foo.unconstrained[0]

def test_iadd(self):
foo = HasLengthConstrainedLists(at_most_five=[1, 2])
foo.at_most_five += [6, 7, 8]
Expand Down Expand Up @@ -1381,6 +1388,13 @@ def test_pop_too_small(self):
foo.at_least_two.pop(10)
self.assertEqual(foo.at_least_two, [1, 2])

def test_pop_from_empty(self):
foo = HasLengthConstrainedLists()
with self.assertRaises(IndexError):
foo.unconstrained.pop()
with self.assertRaises(IndexError):
foo.unconstrained.pop(10)

def test_remove(self):
foo = HasLengthConstrainedLists(at_least_two=[1, 2, 6, 4])
foo.at_least_two.remove(2)
Expand All @@ -1398,6 +1412,11 @@ def test_remove_too_small(self):
foo.at_least_two.remove(10)
self.assertEqual(foo.at_least_two, [1, 2])

def test_remove_from_empty(self):
foo = HasLengthConstrainedLists()
with self.assertRaises(ValueError):
foo.unconstrained.remove(35)

def test_length_violation_error_message(self):
# Regression test for enthought/traits#1170
foo = HasLengthConstrainedLists(at_least_two=[1, 2])
Expand Down
6 changes: 3 additions & 3 deletions traits/trait_list_object.py
Original file line number Diff line number Diff line change
Expand Up @@ -634,7 +634,7 @@ def __delitem__(self, key):
"""

removed_count = len(self[key]) if isinstance(key, slice) else 1
self._validate_length(len(self) - removed_count)
self._validate_length(max(len(self) - removed_count, 0))
super().__delitem__(key)

def __iadd__(self, value):
Expand Down Expand Up @@ -776,7 +776,7 @@ def pop(self, index=-1):
If list is empty or index is out of range.
"""

self._validate_length(len(self) - 1)
self._validate_length(max(len(self) - 1, 0))
return super().pop(index)

def remove(self, value):
Expand All @@ -797,7 +797,7 @@ def remove(self, value):
If the value is not present.
"""

self._validate_length(len(self) - 1)
self._validate_length(max(len(self) - 1, 0))
super().remove(value)

# -- pickle and copy support ----------------------------------------------
Expand Down

0 comments on commit 4dca1f2

Please sign in to comment.