X Tutup
Skip to content

Fix qt5 mouse#8440

Merged
efiring merged 6 commits intomatplotlib:v2.0.xfrom
tacaswell:fix_qt5_mouse
Apr 24, 2017
Merged

Fix qt5 mouse#8440
efiring merged 6 commits intomatplotlib:v2.0.xfrom
tacaswell:fix_qt5_mouse

Conversation

@tacaswell
Copy link
Copy Markdown
Member

@tacaswell tacaswell commented Apr 7, 2017

attn @efiring

This is the last thing for 2.0.1 I believe.

@tacaswell tacaswell added the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label Apr 7, 2017
@tacaswell tacaswell added this to the 2.0.1 (next bug fix release) milestone Apr 7, 2017
@tacaswell tacaswell changed the base branch from master to v2.0.x April 7, 2017 03:42
@tacaswell tacaswell closed this Apr 7, 2017
@tacaswell tacaswell reopened this Apr 7, 2017
@QuLogic
Copy link
Copy Markdown
Member

QuLogic commented Apr 7, 2017 •
edited
Loading

So this basically undoes #8144, but that's supposedly fixing a bug. Is there some difference in Qt5 versions being used or something?

@efiring
Copy link
Copy Markdown
Member

efiring commented Apr 7, 2017

This is what I am using when seeing the bug:

(test) efiring@manini2:~/currents/programs/pycurrents/scripts$ conda list pyqt
# packages in environment at /Users/efiring/anaconda/envs/test:
#
pyqt                      5.6.0                    py35_0

@QuLogic
Copy link
Copy Markdown
Member

QuLogic commented Apr 7, 2017

What about qt?

@efiring
Copy link
Copy Markdown
Member

efiring commented Apr 7, 2017 

(test) efiring@manini2:~/currents/programs/pycurrents/scripts$ conda list | grep -i qt
ipython-qtconsole         4.0.1                    py35_0
pyqt                      5.6.0                    py35_0
qt                        5.6.0                         0
qtawesome                 0.3.3                    py35_0    conda-forge
qtconsole                 4.2.1                    py35_1
qtpy                      1.1.2                    py35_0

@tacaswell
Copy link
Copy Markdown
Member Author

tacaswell commented Apr 7, 2017

@QuLogic I think the bug was in @astrofrog 's example code. Internally we need to use the the pixels the renderer thinks it has not the pixels qt thinks it has.

@tacaswell tacaswell mentioned this pull request Apr 7, 2017
Closed
@tacaswell
Copy link
Copy Markdown
Member Author

tacaswell commented Apr 8, 2017

attn @astrofrog Can you test this out with both qt5 and osx?

@tacaswell
Copy link
Copy Markdown
Member Author

tacaswell commented Apr 8, 2017

I still suspect that there are a bunch of un-needed scaling factors in there (as we scale up one place and down in the other), but it is not clear exactly which pairs of them need to be pulled out.

@tacaswell
Copy link
Copy Markdown
Member Author

tacaswell commented Apr 8, 2017

And I am working on a "high-dpi" style sheet.

@astrofrog
Copy link
Copy Markdown
Contributor

astrofrog commented Apr 8, 2017

@tacaswell - can you mark me as a reviewer so that I don't lose track of this, and I'll review/test it on Monday

@tacaswell tacaswell requested a review from astrofrog April 8, 2017 23:40
@tacaswell tacaswell force-pushed the fix_qt5_mouse branch 2 times, most recently from 9e34497 to 5e869de Compare April 10, 2017 20:06
@efiring
Copy link
Copy Markdown
Member

efiring commented Apr 12, 2017

I deleted a comment about this hanging my terminal; it seems to have been caused by an ipython version update.

@stonebig stonebig mentioned this pull request Apr 15, 2017
Closed
@efiring
Copy link
Copy Markdown
Member

efiring commented Apr 17, 2017

@tacaswell, your suggestion of adding back the block that was deleted at the end of cedaaae works fine; I think that with that change, this is ready for 2.0.1.

@tacaswell
FIX: internally Matplotlib works in screen pixels not logical pixels
a40620a
@tacaswell
MNT: tweak the toolbar for Qt
39b86d6 
Do not over-size the icons
@tacaswell
MNT: Always use scaling with Qt if available
5a6826f
@tacaswell
ENH: setting the DPI properly propagates
d739943 
Set the figure size back to the current value to push the dpi (and
hence rendered size) change through.
@tacaswell
TST: add test for dpi change
533f0bb
@tacaswell
FIX: fix up savefig dpi if scaled for screen
278e8ef 
If `_original_dpi` is tacked on to the figure instance by a backend
canvas use that for 'figure' dpi instead of the dpi scaled for the
screen.
@astrofrog
Copy link
Copy Markdown
Contributor

astrofrog commented Apr 17, 2017

Sorry I dropped the ball on reviewing this, I won't have time to check it in the short term but I don't want to delay 2.0.1, so this is fine by me.

@tacaswell
Copy link
Copy Markdown
Member Author

tacaswell commented Apr 18, 2017

@efiring Can you test this set of commits (I force-pushed).

There is one more thing I want to add (but might not be worth holding PR over) is to connect up the signal Qt provides when the dpi ratio changes.

@efiring
Copy link
Copy Markdown
Member

efiring commented Apr 18, 2017

In my quick check, it looks good and works perfectly. I tested saving to png via the gui, manual resizing, get_size_inches and set_size_inches, cursor readout, and zoom to rect.

@efiring efiring merged commit 7d9036c into matplotlib:v2.0.x Apr 24, 2017
@tacaswell tacaswell deleted the fix_qt5_mouse branch April 24, 2017 19:19
@cgohlke cgohlke mentioned this pull request May 2, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@astrofrog astrofrog Awaiting requested review from astrofrog

Assignees

No one assigned

Labels

Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.

Projects

None yet

Milestone

v2.0.1

Development

Successfully merging this pull request may close these issues.

4 participants

@tacaswell @QuLogic @efiring @astrofrog
X Tutup