-
-
Notifications
You must be signed in to change notification settings - Fork 153
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
PR: Make warning usage consistant and refine messages #398
PR: Make warning usage consistant and refine messages #398
Conversation
(Oops, fell asleep before I went back and hit the ready for review button) |
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.
Two small suggestions for you @CAM-Gerlach, otherwise looks good to me.
826bb88
to
88c0bc6
Compare
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.
Looks good to me now, thanks @CAM-Gerlach!
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.
Thanks @CAM-Gerlach !
Also @CAM-Gerlach should this close #395 ? |
It may help with it some, but it isn't clear that it would resolve the actual issue the user experienced, since from the very limited information provided it is not at all clear what that actually is. I suggest we wait at least a couple weeks to see if they respond. Also, you mentioned documenting the error/warning hierarchy somewhere, which is a good idea to do at some point. |
Currently, as noted in issue #395 , a generic
RuntimeWarning
is emitted when falling back to a non-selected binding, instead of QtPy's ownPythonQtWarning
subclass. Furthermore, the text could perhaps be clarified a bit. I also noticed a few other issues when making this changed, which I fixed.Therefore, this PR:
PythonQtWarning
instead of a genericRuntimeWarning
when falling back to an unselected binding, consistent with the other warnings and errors that use QtPy-specific subclassesPythonQtWarning
inherit fromRuntimeWarning
instead of a genericWarning
, to be consistent with all the other errors and warnings and with whatRuntimeWarning
is meant for, and ensure the previous replacement is fully backwards compatiblePythonQtWarning
andPythonQtError
!r
instead of hardcoded quotes