Fix KeyError from observe trying to filter events for trait named "*_items" #1328
Conversation
kitchoi
changed the title
| @@ -129,8 +129,7 @@ def ctrait_prevent_event(event): | |||
| """ | |||
| if event.old is Uninitialized: | |||
| return True | |||
|
|
|||
| ctrait = event.object.traits()[event.name] | |||
| ctrait = event.object._trait(event.name, 2) | |||
There was a problem hiding this comment.
The 2 here forces instance trait creation, which seems like a potentially surprising side-effect. Is this necessary? If so, should the docstring be updated to indicate the potential side-effect?
There was a problem hiding this comment.
Would using the event.object.trait method be an option here?
There was a problem hiding this comment.
The 2 here forces instance trait creation, which seems like a potentially surprising side-effect. Is this necessary?
This function is called when the event has already fired from the CTrait. So persumably the ctrait should have been created already?
There was a problem hiding this comment.
Anyway, yes I can use trait method here.
There was a problem hiding this comment.
persumably the ctrait should have been created already
Not sure. Could there be a class trait but not an instance trait at this point? Or are we sure that there already has to be an instance trait?
There was a problem hiding this comment.
I don't know - I was asking mainly for my understanding. I had the understanding that change handlers are always attached to an instance CTrait, I don't know if that is right.
| @@ -129,8 +129,7 @@ def ctrait_prevent_event(event): | |||
| """ | |||
| if event.old is Uninitialized: | |||
| return True | |||
|
|
|||
| ctrait = event.object.traits()[event.name] | |||
| ctrait = event.object.trait(event.name) | |||
There was a problem hiding this comment.
Just as a highly personal nitpicky matter of style, I do like the blank line following the early return.
There was a problem hiding this comment.
It was accidental, but then I was lazy to revert, sorry about that... reverting.
This PR fixes a new bug introduced by #1165 (hence not released).
The first commit adds a test that reveals the issue, it fails with the following error:
The second commit fixes it.
Checklist
Update API reference (docs/source/traits_api_reference)Update User manual (docs/source/traits_user_manual)Update type annotation hints intraits-stubs