Fix TraitError when mutating a list/dict/set inside another container

[kitchoi]

Fix the TraitError when:

• mutating a list nested inside Dict(Any, Union(None, List))
• mutating a dict nested inside a list
• mutating a set nested inside a list, or a dict

Behaviour change:

• Mutation on a nested container no longer fires events under the name of the outer-most container. e.g. previously mutating the inner list in list_of_list = List(List) will fire an item change on list_of_list_items. Similarly for Dict(Any, Dict).

Closes #25

Note that this does NOT resolve the same error seen when Set and Dict is wrapped inside Either in #278. That error is caused by something else.

EDITED: The issue was closed as a duplicate of #25, but this PR does NOT close #25 because the same fix has not been applied to TraitDictObject, which is currently being refactored in #913.

Also thanks to @midhun-pm for his previous work related to this, on #951

Checklist

• Tests
• Update API reference (docs/source/traits_api_reference)
• Update User manual (docs/source/traits_user_manual)
• Update type annotation hints in traits-stubs

[kitchoi]

Note that #951 was closed under the assumption that the issue would be closed by #977

However that is in fact not true as long as TraitListObject.notify (which raises the error) stays for the support of on_trait_change.

[ievacerny]

This is not a long fix, so maybe it would be good to add the fix to Set and Dict implementations in this PR as well?

[kitchoi]

This is not a long fix, so maybe it would be good to add the fix to Set and Dict implementations in this PR as well?

Happy to do that, then we close issue #25 in one go!

[kitchoi]

I have expanded the fix for TraitDictObject and TraitSetObject. I also added quite a number of tests.

All the added tests failed on the commit before #913 was merged:

Test failures at c91e91e

======================================================================
ERROR: test_modify_set_in_dict_no_events (traits.tests.test_regression.TestRegressionNestedContainerEvent)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/kchoi/Work/ETS/traits/traits/tests/test_regression.py", line 417, in test_modify_set_in_dict_no_events
    instance.dict_of_set["1"].add(1)
  File "/Users/kchoi/Work/ETS/traits/traits/trait_set_object.py", line 267, in add
    self.notify(set(), {value})
  File "/Users/kchoi/Work/ETS/traits/traits/trait_set_object.py", line 133, in notify
    notifier(removed, added)
  File "/Users/kchoi/Work/ETS/traits/traits/trait_set_object.py", line 534, in notifier
    object.trait_items_event(self.name_items, event, items_event)
  File "/Users/kchoi/Work/ETS/traits/traits/base_trait_handler.py", line 76, in error
    raise TraitError(
traits.trait_errors.TraitError: The 'dict_of_set_items' trait of a NestedContainerClass instance must be a TraitDictEvent or None, but a value of TraitSetEvent(removed=set(), added={1}) <class 'traits.trait_set_object.TraitSetEvent'> was specified.

======================================================================
ERROR: test_modify_set_in_union_in_dict (traits.tests.test_regression.TestRegressionNestedContainerEvent)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/kchoi/Work/ETS/traits/traits/tests/test_regression.py", line 428, in test_modify_set_in_union_in_dict
    instance.dict_of_union_none_or_set["1"].add(1)
  File "/Users/kchoi/Work/ETS/traits/traits/trait_set_object.py", line 267, in add
    self.notify(set(), {value})
  File "/Users/kchoi/Work/ETS/traits/traits/trait_set_object.py", line 133, in notify
    notifier(removed, added)
  File "/Users/kchoi/Work/ETS/traits/traits/trait_set_object.py", line 534, in notifier
    object.trait_items_event(self.name_items, event, items_event)
  File "/Users/kchoi/Work/ETS/traits/traits/base_trait_handler.py", line 76, in error
    raise TraitError(
traits.trait_errors.TraitError: The 'dict_of_union_none_or_set_items' trait of a NestedContainerClass instance must be a TraitDictEvent or None, but a value of TraitSetEvent(removed=set(), added={1}) <class 'traits.trait_set_object.TraitSetEvent'> was specified.

======================================================================
FAIL: test_modify_dict_in_dict_does_not_fire (traits.tests.test_regression.TestRegressionNestedContainerEvent)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/kchoi/Work/ETS/traits/traits/tests/test_regression.py", line 385, in test_modify_dict_in_dict_does_not_fire
    self.assertEqual(len(self.events), 0)
AssertionError: 1 != 0

======================================================================
FAIL: test_modify_dict_in_list (traits.tests.test_regression.TestRegressionNestedContainerEvent)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/kchoi/Work/ETS/traits/traits/tests/test_regression.py", line 365, in test_modify_dict_in_list
    instance.list_of_dict[0]["key"] = 1
traits.trait_errors.TraitError: Each value of the 'list_of_dict_items' trait of a NestedContainerClass instance must be a TraitListEvent or None, but a value of TraitDictEvent(added={'key': 1}, changed={}, removed={}) <class 'traits.trait_dict_object.TraitDictEvent'> was specified.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/kchoi/Work/ETS/traits/traits/tests/test_regression.py", line 367, in test_modify_dict_in_list
    self.fail("Mutating a nested dict should not fail.")
AssertionError: Mutating a nested dict should not fail.

======================================================================
FAIL: test_modify_dict_in_list_with_new_value (traits.tests.test_regression.TestRegressionNestedContainerEvent)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/kchoi/Work/ETS/traits/traits/tests/test_regression.py", line 374, in test_modify_dict_in_list_with_new_value
    instance.list_of_dict[-1]["key"] = 1
traits.trait_errors.TraitError: Each value of the 'list_of_dict_items' trait of a NestedContainerClass instance must be a TraitListEvent or None, but a value of TraitDictEvent(added={'key': 1}, changed={}, removed={}) <class 'traits.trait_dict_object.TraitDictEvent'> was specified.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/kchoi/Work/ETS/traits/traits/tests/test_regression.py", line 376, in test_modify_dict_in_list_with_new_value
    self.fail("Mutating a nested dict should not fail.")
AssertionError: Mutating a nested dict should not fail.

======================================================================
FAIL: test_modify_dict_in_union_in_dict (traits.tests.test_regression.TestRegressionNestedContainerEvent)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/kchoi/Work/ETS/traits/traits/tests/test_regression.py", line 396, in test_modify_dict_in_union_in_dict
    self.assertEqual(len(self.events), 0)
AssertionError: 1 != 0

======================================================================
FAIL: test_modify_list_in_dict (traits.tests.test_regression.TestRegressionNestedContainerEvent)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/kchoi/Work/ETS/traits/traits/tests/test_regression.py", line 341, in test_modify_list_in_dict
    instance.dict_of_list["name"].append("word")
traits.trait_errors.TraitError: The 'dict_of_list_items' trait of a NestedContainerClass instance must be a TraitDictEvent or None, but a value of TraitListEvent(index=0, removed=[], added=['word']) <class 'traits.trait_list_object.TraitListEvent'> was specified.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/kchoi/Work/ETS/traits/traits/tests/test_regression.py", line 343, in test_modify_list_in_dict
    self.fail("Mutating a nested list should not fail.")
AssertionError: Mutating a nested list should not fail.

======================================================================
FAIL: test_modify_list_in_dict_wrapped_in_union (traits.tests.test_regression.TestRegressionNestedContainerEvent)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/kchoi/Work/ETS/traits/traits/tests/test_regression.py", line 350, in test_modify_list_in_dict_wrapped_in_union
    instance.dict_of_union_none_or_list["name"].append("word")
traits.trait_errors.TraitError: The 'dict_of_union_none_or_list_items' trait of a NestedContainerClass instance must be a TraitDictEvent or None, but a value of TraitListEvent(index=0, removed=[], added=['word']) <class 'traits.trait_list_object.TraitListEvent'> was specified.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/kchoi/Work/ETS/traits/traits/tests/test_regression.py", line 352, in test_modify_list_in_dict_wrapped_in_union
    self.fail("Mutating a nested list should not fail.")
AssertionError: Mutating a nested list should not fail.

======================================================================
FAIL: test_modify_list_in_list_no_events (traits.tests.test_regression.TestRegressionNestedContainerEvent)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/kchoi/Work/ETS/traits/traits/tests/test_regression.py", line 359, in test_modify_list_in_list_no_events
    self.assertEqual(len(self.events), 0, "Expected no events.")
AssertionError: 1 != 0 : Expected no events.

======================================================================
FAIL: test_modify_set_in_list (traits.tests.test_regression.TestRegressionNestedContainerEvent)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/kchoi/Work/ETS/traits/traits/tests/test_regression.py", line 401, in test_modify_set_in_list
    instance.list_of_set[0].add(1)
traits.trait_errors.TraitError: The 'list_of_set_items' trait of a NestedContainerClass instance must be a TraitListEvent or None, but a value of TraitSetEvent(removed=set(), added={1}) <class 'traits.trait_set_object.TraitSetEvent'> was specified.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/kchoi/Work/ETS/traits/traits/tests/test_regression.py", line 403, in test_modify_set_in_list
    self.fail("Mutating a nested set should not fail.")
AssertionError: Mutating a nested set should not fail.

======================================================================
FAIL: test_modify_set_in_list_with_new_value (traits.tests.test_regression.TestRegressionNestedContainerEvent)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/kchoi/Work/ETS/traits/traits/tests/test_regression.py", line 409, in test_modify_set_in_list_with_new_value
    instance.list_of_set[0].add(1)
traits.trait_errors.TraitError: The 'list_of_set_items' trait of a NestedContainerClass instance must be a TraitListEvent or None, but a value of TraitSetEvent(removed=set(), added={1}) <class 'traits.trait_set_object.TraitSetEvent'> was specified.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/kchoi/Work/ETS/traits/traits/tests/test_regression.py", line 411, in test_modify_set_in_list_with_new_value
    self.fail("Mutating a nested set should not fail.")
AssertionError: Mutating a nested set should not fail.

----------------------------------------------------------------------

At the commit when #913 was merged, these tests pass:

test_modify_dict_in_dict_does_not_fire
test_modify_list_in_dict
test_modify_set_in_dict_no_events

That state is the same with the current master.

Note that I did not solve the Either(None, Dict) error here (see #281 (comment)). But I did manage to fix it by modifying this function:

traits/traits/trait_handlers.py

Lines 1127 to 1135 in 77cbf0a

	def items_event():
	from .trait_types import Event
	if TraitList._items_event is None:
	TraitList._items_event = Event(
	TraitListEvent, is_base=False
	).as_ctrait()
	return TraitList._items_event

I'd prefer doing that separately because the cause is different, even though the symptom looks the same.

[kitchoi]

I also updated the main description for this PR.

[kitchoi]

This is the bit that worries me:

Mutation on a nested container no longer fires events under the name of the outer-most container. e.g. previously mutating the inner list in list_of_list = List(List) will fire an item change on list_of_list_items. Similarly for Dict(Any, Dict).

The (original) behaviour is considered a bug, but it is also conceivable that it is a bug being exploited. I am going to run this branch against a sizeable project and see if this breaks things.

[ievacerny]

Thanks for implementing the fix for all three traits!

The code and the tests LGTM apart from a couple minor comments.

[kitchoi]

The (original) behaviour is considered a bug, but it is also conceivable that it is a bug being exploited. I am going to run this branch against a sizeable project and see if this breaks things.

Nothing broke for the project I tried. :)

From offline discussion, this existing behaviour is a documented bug so we should go ahead to fix. Prior to the release, we will assess whether this causes any massive, hard-to-fix breakage. So far there are no evidence that this will.

[kitchoi]

Thank you @ievacerny! Merging...