-
Notifications
You must be signed in to change notification settings - Fork 55
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
Remove 'parent' argument from wx StatusBarManager #1192
Remove 'parent' argument from wx StatusBarManager #1192
Conversation
Remove the 'parent' argument from 'StatusBarManager.destroy' in the WX implementation. The argument is unused, and doesn't match the interface definition.
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.
Once again, thanks for the PR. This probably needs a corresponding change to the call here:
pyface/pyface/ui/wx/application_window.py
Line 225 in acccc5e
old.destroy(self.control) |
Good catch, I've adapted the call site now. I wonder if we should add some kind of test? Originally, I stumbled upon this because our code triggered the following exception:
|
There are tests for the create/destroy aspects of the pyface/pyface/tests/test_application_window.py Lines 222 to 252 in 9aeb3bd
Unfortunately those tests need the So the long and short of it is, no, no need to write a test, and this looks good to go. |
Thanks for the review, and the context on testing! |
Another small fix:
Remove the 'parent' argument from 'StatusBarManager.destroy' in the wx backend. The argument is unused, and doesn't match the interface definition.
If you prefer keeping the argument (to not break applications which may pass it explicitly?), I'd propose adding a default
=None
, and emitting a deprecation warning.