FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

FIX: update spine positions before get extents by jklymak · Pull Request #11754 · matplotlib/matplotlib · GitHub

  1. matplotlib
  2. matplotlib

Repository navigation

FIX: update spine positions before get extents - #11754

Merged
dstansby merged 4 commits into
matplotlib:masterfrom
jklymak:fix-update-spine-positions-before-getting-extents
Jul 26, 2018
ConversationCommits (4)ChecksFiles changed
Merged

FIX: update spine positions before get extents#11754
dstansby merged 4 commits into
matplotlib:masterfrom
jklymak:fix-update-spine-positions-before-getting-extents

Conversation

jklymak commented Jul 23, 2018 •
edited
Loading

Copy link
Copy Markdown
Member

PR Summary

Closes #11737 (supercedes #11739, #11742)

If you zoomed on an axes, get_tightbbox would return a huge bounding box because spine objects didn't have their x/ylims properly set from the axis info yet. This PR sets those limits when get_window_extent is called making the bounding box accurate..

This also rolls back a change in #11627 where constrained_layout was turned off under ZOOM and PAN...

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 8 compliant

jklymak mentioned this pull request Jul 23, 2018
Closed
6 tasks
jklymak added status: work in progress Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. labels Jul 23, 2018
jklymak added this to the v3.0 milestone Jul 23, 2018
jklymak changed the title FIX: update spine positions before gett extents FIX: update spine positions before get extents Jul 23, 2018

fredrik-1 commented Jul 23, 2018

Copy link
Copy Markdown
Contributor

Seems to work, thanks!

jklymak commented Jul 23, 2018

Copy link
Copy Markdown
Member Author

@fredrik-1 Great! And thanks for catching this before 3.0 went out. Now to devise a test that fails if this doesn't work...

tacaswell reviewed Jul 23, 2018
View reviewed changes
Comment thread lib/matplotlib/spines.py Outdated
# make sure the location is updated so that transforms etc are
# correct:
self._adjust_location()
return self.get_path().get_extents(self.get_transform())

tacaswell Jul 23, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

super().get_window_extent(renderer=renderer) instead?

jklymak mentioned this pull request Jul 23, 2018
Closed
6 tasks

jklymak commented Jul 23, 2018

Copy link
Copy Markdown
Member Author

...test done. The test fails on master..

jklymak commented Jul 23, 2018
View reviewed changes
Comment thread lib/matplotlib/tests/test_axes.py Outdated

@image_comparison(baseline_images=['csd_freqs'], remove_text=True,
extensions=['png'])
extensions=['png'], tol=0.002)

jklymak Jul 23, 2018

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This fix is for the macOS test and doesn't affect this PR. Happy to remove it if the commits will be cleaner.

dstansby Jul 24, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Can you throw this in another PR? It would be good to get to the bottom of the cause (I'll investigate later today) before just adding a tolerance.

jklymak Jul 24, 2018

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Removed from this PR. I'll not open a new PR in lieu of you tracking down the problem (I assume you have a Mac?) If you want me to play around w/ np1.15 I can also give that a shot. Note that I don't get the test failure on my mac w/ np1.14.

jklymak mentioned this pull request Jul 24, 2018
Closed
6 tasks
jklymak force-pushed the fix-update-spine-positions-before-getting-extents branch from 0ab9951 to 5545050 Compare July 24, 2018 17:23

jklymak commented Jul 24, 2018

Copy link
Copy Markdown
Member Author

The only failure is the Mac CI small 0.001 tolerance issue w/ csd caused by numpy1.15

dstansby merged commit cb47f2d into matplotlib:master Jul 26, 2018
jklymak deleted the fix-update-spine-positions-before-getting-extents branch July 26, 2018 16:34
tacaswell mentioned this pull request Jun 2, 2023
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

tacaswell tacaswell approved these changes
dstansby dstansby approved these changes

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

v3.0.0

Development

Successfully merging this pull request may close these issues.

Bug in tight_layout

4 participants

Back | FazBrowse Home | New Git URL