-
Notifications
You must be signed in to change notification settings - Fork 44
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
Update super usage in potentially buggy cases #790
Conversation
we were using `super(Container, self)` from inside `Canvas`, which seems like an unintentional bug
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
@@ -53,7 +53,7 @@ class GuiTestAssistant(object): | |||
class HoverToolTestCase(EnableTestAssistant, GuiTestAssistant, | |||
unittest.TestCase): | |||
def setUp(self): | |||
super(HoverToolTestCase, self).setUp() | |||
GuiTestAssistant.setUp(self) |
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.
Same question I had on the other PR, why exactly do we purposefully avoid super for setUp
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.
answered elsewhere - part convention, part best practice - especially because we have cases where we need to call setUp
on more than one base class.
See related #789
This PR updates a few calls to
super
that were potentially buggy. In these two cases for example, we were callingsuper(A, self)
from classB
whereB
inherits fromA
. We first update thesuper
calls to use the correct class argument - and then update thesuper
call to remove the arguments altogether.