Implement DictItemObserver for observing mutations to a dict #1072
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1072 +/- ##
==========================================
- Coverage 76.15% 73.37% -2.79%
==========================================
Files 54 69 +15
Lines 6493 8213 +1720
Branches 1263 1568 +305
==========================================
+ Hits 4945 6026 +1081
- Misses 1205 1808 +603
- Partials 343 379 +36
Continue to review full report at Codecov.
|
|
Merge conflicts are resulting from API modules and API documentation source shared with other PRs. |
…ll. Remove protection to be consistent with ListItemObserver post-review.
|
|
||
|
|
||
| class DictChangeEvent: | ||
| """ Event object to represent mutations on a dict. |
There was a problem hiding this comment.
To avoid accidents, I think it's worth adding a explicit sentence here about the difference between this API and the removed / added / changed API. Otherwise I think people will glance at this, and just assume that it behaves like the existing API.
| def __eq__(self, other): | ||
| """ Return true if this observer is equal to the given one.""" | ||
| return ( | ||
| type(self) is type(other) |
There was a problem hiding this comment.
We're still inconsistent across the codebase about using type (e.g., as here) versus using .__class__ (e.g., in the __repr__ for DictChangeEvent). They're subtly different, and some of the new typing machinery being introduced in recent Python versions is enlarging that difference (e.g., in Python 3.9, list[int] has a different __class__ from its type).
I think we actually want to use __class__ consistently throughout the codebase, despite the general rule that one shouldn't generally use dunder attributes and methods directly as a consumer.
Not something that should be changed in this PR; just something to think about.
|
|
||
|
|
||
| def create_observer(**kwargs): | ||
| """ Convenient function for creating DictItemObserver with default values. |
|
Still LGTM, modulo conflicts. |
|
Thanks! I put this side-by-side with the List counterpart and they look consistent as far as I can tell. Will merge when CI is green. |
I knew I am going to spot inconsistencies later, and I did find one after merge. |
Here it is: #1085 |
This PR implements item 9 of #977:
DictChangeEvent. Note that the definition ofDictChangeEventis not the same asTraitDict.notify, see discussion in Remove "changed" from TraitDict change event #1031DictItemObserverthat implements the interface ofIObserver:- return the object for adding/removing notifiers. In this case, the observable is simply an instance of TraitDict.
- yield the values of the dict for the next observer(s) in an ObserverGraph. values is an obvious choice here because mutables should not be used as keys in dict.
- Like
ListItemObserver, no filtering is done while yielding values for the next observer(s) on the observer graph. If the values are incompatible with the next observers, the latter can complain.Checklist
docs/source/traits_api_reference)Update User manual (docs/source/traits_user_manual)Update type annotation hints intraits-stubs