-
Notifications
You must be signed in to change notification settings - Fork 85
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix more missing mentions of Set in notification docs #1618
Conversation
|
||
.. index:: old parameter to the notification handlers | ||
|
||
* *old*: The old value of the trait attribute that changed. For changes to List | ||
and Dict object, this is a list of items that were deleted. For event traits, | ||
and Dict objects, this is a list of items that were deleted. For changes to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure that this is right for Dict
; I need to see if I can reproduce.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, I can't find any way to reproduce behaviour that would be consistent with this for Dict
objects.
For example, this fails to invoke the handler:
Python 3.10.3 (main, Mar 17 2022, 16:03:18) [Clang 13.0.0 (clang-1300.0.29.3)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from traits.api import *
>>> class A(HasTraits):
... foo = Dict()
...
>>> def handler(object, name, old, new): print(object, name, old, new)
...
>>> a = A()
>>> a.on_trait_change(handler, "foo[]")
>>> a.foo[4] = 16
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... and the same is true in Traits 5.2.0, so this isn't one of the many things that we broke when introducing the new list, dict and set implementations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM modulo working out what is going on with Dicts and []
This PR builds on #1617 to make more extensive updates and fixes to the
on_trait_change
documentation:List
andDict
also apply toSet
; fix missing mentions ofSet
trait_name[]
pattern applies to bothList
andSet
(but notDict
); fix docs accordingly