-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Fix handling of getSaveFileName to be consistent [backport to 1.4.x] #3469
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
Conversation
PyQt4 and PyQt5 handle getSaveFileName differently, returning either a filename or a filename,filter tuple respectively. PySide behaves as for PyQt5. This caused bug #3454 producing an 'format not supported' error as the tuple did not match any of the string types. This updates each API to return a tuple as per PyQt5 (following the principle of using the latest API as a target). For recent PyQt4 this means using getSaveFileNameAndFilter instead, for older PyQt4 we wrap the function to output a tuple. Figure saving has been tested on PySide, PyQt4 and PyQt5.
There is no back-compatibility issue here because this is all private details and anyone using them knew the risks. |
And it looks like there are a couple of pep8 issues
|
@tacaswell what you do mean by "There is no back-compatibility issue here because this is all private details and anyone using them knew the risks."? The changes don't attempt backwards compatibility just update the backends to correctly follow the "do it like PyQt5" (+PySide coincidentally) approach. Were you thinking if others may be using the matplotlib I've fixed the PEP8 compliance issues and added another commit. I can combine them into one if needed? |
I think he was merely making a note to himself, reminding himself that this On Fri, Sep 5, 2014 at 4:15 AM, Martin Fitzpatrick <notifications@github.com
|
Thanks @WeatherGod. It's worth noting then @tacaswell that there is no need to break back-compatibility if it is an issue, as the wrapper can simply be written to look like the old-style. I changed it purely for the sake of staying as-close-as-possible to the latest API version, but as it is a private API feature that may not be considered important. |
@WeatherGod hit it on the head. My first reaction was that this broke 'public' api (I at least use the |
Fixed pep8 and merged to master locally
Fixed pep8 and merged to master locally
@mfitzp Thanks! Good luck with the writing and the move. I did that about 9 months ago. |
PyQt4 and PyQt5 handle getSaveFileName differently, returning either
a filename or a filename,filter tuple respectively. PySide behaves as
for PyQt5. This caused bug #3454 producing an 'format not supported'
error as the tuple did not match any of the string types.
This updates each API to return a tuple as per PyQt5 (following the
principle of using the latest API as a target). For recent PyQt4
this means using getSaveFileNameAndFilter instead, for older PyQt4
we wrap the function to output a tuple.
Figure saving has been tested on PySide, PyQt4 and PyQt5.