Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

FIX: maybe improve renderer dance#22732

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
tacaswell merged 1 commit intomatplotlib:mainfromjklymak:fix-renderer-dance
Apr 4, 2022

Conversation

jklymak
Copy link
Member

@jklymakjklymak commentedMar 30, 2022
edited
Loading

PR Summary

Closes#22673

tight_layout andconstrained_layout were both callingbackend_bases._get_renderer. However, that was probably too low-level, and seems to cause canvas-dpi related flakiness as in#22673, in particular some users were seeing changed dpi becausesavefig.format was set to a vector backend.

This uses the more robust algorithm in_tight_layout.get_renderer. In particular iffig.canvas already exists and has a renderer, use that because it means the user has already (perhaps interactively) created a canvas. This stops prematurely falling back to a default canvas indicated bysavefig.format. This probablymostly worked for folks because the defaultformat ispng which is enough like on-screen display that things didn't break.

PR Checklist

Tests and Styling

  • Has pytest style unit tests (andpytest passes).
  • IsFlake 8 compliant (installflake8-docstrings and runflake8 --docstring-convention=all).

Documentation

  • New features are documented, with examples if plot related.
  • New features have an entry indoc/users/next_whats_new/ (follow instructions in README.rst there).
  • API changes documented indoc/api/next_api_changes/ (follow instructions in README.rst there).
  • Documentation is sphinx and numpydoc compliant (the docs shouldbuild without error).

@jklymakjklymak added this to thev3.5.2 milestoneMar 30, 2022
@jklymakjklymak added topic: geometry managerLayoutEngine, Constrained layout, Tight layout Release criticalFor bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. labelsMar 30, 2022
@jklymak
Copy link
MemberAuthor

BTW, I have no real way to test this, other than the existing tests, which pass....

@jklymakjklymak marked this pull request as ready for reviewMarch 30, 2022 15:34
@anntzer
Copy link
Contributor

I agree that this is better; likely this "use the cached renderer" logic should always be used even in backend_bases._get_renderer as long as print_method is None (i.e. we're not in a context of savefig with a known format)? That can be fixed lated, though.

Copy link
Member

@tacaswelltacaswell left a comment

Choose a reason for hiding this comment

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

Seems a bit odd that everything is routed through tight_layout, but this closes at least 2 issues.

@tacaswelltacaswell merged commit9e0eb54 intomatplotlib:mainApr 4, 2022
@lumberbot-app
Copy link

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

  1. Checkout backport branch and update it.
git checkout v3.5.xgit pull
  1. Cherry pick the first parent branch of the this PR on top of the older branch:
git cherry-pick -x -m1 9e0eb54fd98bb804f51a74101db88164870a2458
  1. You will likely have some merge/cherry-pick conflict here, fix them and commit:
git commit -am 'Backport PR #22732: FIX: maybe improve renderer dance'
  1. Push to a named branch:
git push YOURFORK v3.5.x:auto-backport-of-pr-22732-on-v3.5.x
  1. Create a PR against branch v3.5.x, I would have named this PR:

"Backport PR#22732 on branch v3.5.x (FIX: maybe improve renderer dance)"

And apply the correct labels and milestones.

Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon!

Remember to remove theStill Needs Manual Backport label once the PR gets merged.

If these instructions are inaccurate, feel free tosuggest an improvement.

@anntzer
Copy link
Contributor

Seems a bit odd that everything is routed through tight_layout

As noted above, probably that logic should be pulled out of TL and moved into backend_bases.

tacaswell reacted with thumbs up emoji

@tacaswell
Copy link
Member

This backport is odd because a bunch of stuff has moved around.....I'm doing it and will make sure the two linked issues still work.

tacaswell added a commit to tacaswell/matplotlib that referenced this pull requestApr 4, 2022
Merge pull requestmatplotlib#22732 from jklymak/fix-renderer-danceFIX: improve cached renderer dance(cherry picked from commit9e0eb54)The changes had to be moved to figure.py due to a major refactoring that willbe released in 3.6 that is already on main.
@jklymakjklymak deleted the fix-renderer-dance branchApril 5, 2022 07:22
@jklymak
Copy link
MemberAuthor

As noted above, probably that logic should be pulled out of TL and moved into backend_bases.

#22745 Moves the logic toFigure. TheFigure holds the cache, so I'm not clear what we gain from putting the logic in the backends?

tacaswell added a commit that referenced this pull requestApr 5, 2022
…-v3.5.xBackport PR#22732: FIX: maybe improve renderer dance
@anntzer
Copy link
Contributor

It doesn't really matter to me whether figure or b_bases hold that logic (well, in my view figure is more just for artists and b_bases for canvas/renderer related stuff, but that's no big deal), mostly the point is that having that in t_l doesn't make much sense.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@tacaswelltacaswelltacaswell approved these changes

@anntzeranntzeranntzer approved these changes

Assignees
No one assigned
Labels
Release criticalFor bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.topic: geometry managerLayoutEngine, Constrained layout, Tight layout
Projects
None yet
Milestone
v3.5.2
Development

Successfully merging this pull request may close these issues.

[Bug]: tight_layout (version 3.5+) [Bug]: EPS savefig messed up by 'figure.autolayout' rcParam on 3.5.0
4 participants
@jklymak@anntzer@tacaswell@QuLogic

[8]ページ先頭

©2009-2025 Movatter.jp