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

Dont double draw#17923

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

Draft
tacaswell wants to merge8 commits intomatplotlib:main
base:main
Choose a base branch
Loading
fromtacaswell:dont-double-draw
Draft

Conversation

tacaswell
Copy link
Member

PR Summary

Replaces#5800

PR Checklist

  • Has Pytest style unit tests
  • Code isFlake 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/next_api_changes/* if API changed in a backward-incompatible way

@jklymak
Copy link
Member

Does this need an API change note?

@dopplershift
Copy link
Contributor

I think the intention is that this is not really user-visible except for performance.

@tacaswell
Copy link
MemberAuthor

Yes, but I needed the PR number to write it ;)

@jklymak
Copy link
Member

Yes, but I needed the PR number to write it ;)

Thatis a problem with that API re-org structure....

@efiring
Copy link
Member

To reduce breakage in user code, would it make sense to changepyplot.draw() so that if not interactive it callsfigure.canvas.draw()?

Copy link
Member

@efiringefiring left a comment

Choose a reason for hiding this comment

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

slight edit, and some questions

@tacaswell
Copy link
MemberAuthor

To reduce breakage in user code, would it make sense to change pyplot.draw() so that if not interactive it calls figure.canvas.draw()?

I am wary of adding more special cases here. The new behavior is now much closer to what the GUI backends do (in whichdraw_idle schedules a draw the next time the GUI event loops spins at some indeterminate time in the future.

@jklymakjklymak marked this pull request as draftAugust 17, 2020 16:25
@tacaswelltacaswell modified the milestones:v3.4.0,v3.5.0Feb 3, 2021
@tacaswelltacaswell marked this pull request as ready for reviewAugust 5, 2021 18:33
@tacaswelltacaswell requested a review fromefiringAugust 5, 2021 18:35
@efiring
Copy link
Member

I like Jody's suggestion to use draw_no_output directly, instead of indirectly via figure.canvas.draw.

Regarding pyplot.draw: the upshot seems to be that it has always been inherently confusing. The docstring implies it is useless--especially as part of pyplot, which is supposed to be the simplest interface. If I understand correctly, this PR makes it even more useless than it was. As in the tests you are changing, what the pyplot user really needs to do is probably draw_no_output, not draw_idle. In that case, this PR is making pyplot.draw more internally consistent, but in all the cases that matter in user code, it is silently making it useless, when before it was useful precisely because of its internal inconsistency.

@tacaswelltacaswell modified the milestones:v3.5.0,v3.6.0Aug 12, 2021
@tacaswell
Copy link
MemberAuthor

Pushed this to 3.6 to keep kicking the can of thinking this through down the road.

I tried doing a blindfig.canvas.draw() ->fig.draw_no_output() conversion in all of the tests and found 3 cases where we were actually relying on exercising the backend render logic so I am not sure it is safe to do that blind conversion (2 of them were expected failures one actually looked at the RGBA from Agg). I could easily believe that some of the tests are showing that the happy paths are happy.

On the bright side there are only ~150 of them.

@tacaswelltacaswell marked this pull request as draftAugust 12, 2021 18:41
@jklymak
Copy link
Member

Certainly I wasn't suggesting we change all of them, at least here - just the new ones you added in this PR (if they work)!

@tacaswell
Copy link
MemberAuthor

Yeah, but it was less work to use sed to change them all :-p

@timhoffmtimhoffm modified the milestones:v3.6.0,unassignedApr 30, 2022
@story645story645 modified the milestones:unassigned,needs sortingOct 6, 2022
@github-actions
Copy link

Since this Pull Request has not been updated in 60 days, it has been marked "inactive." This does not mean that it will be closed, though it may be moved to a "Draft" state. This helps maintainers prioritize their reviewing efforts. You can pick the PR back up anytime - please ping us if you need a review or guidance to move the PR forward! If you do not plan on continuing the work, please let us know so that we can either find someone to take the PR over, or close it.

@github-actionsgithub-actionsbot added the status: inactiveMarked by the “Stale” Github Action labelAug 11, 2023
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@jklymakjklymakjklymak left review comments

@efiringefiringAwaiting requested review from efiring

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Projects
None yet
Milestone
future releases
Development

Successfully merging this pull request may close these issues.

7 participants
@tacaswell@jklymak@dopplershift@efiring@story645@timhoffm@mdboom

[8]ページ先頭

©2009-2025 Movatter.jp