Skip to content

FIX: don't close figures if switch_backend is a no-op #14471

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

Merged
merged 3 commits into from
Oct 23, 2022

Conversation

tacaswell
Copy link
Member

I think this should go in for 3.1.1 as it helps to reduce the
action-at-a-distance issues around the auto-backend / IPython
configuration.

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

anntzer
anntzer previously approved these changes Jun 6, 2019
Copy link
Contributor

@anntzer anntzer left a comment

Choose a reason for hiding this comment

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

modulo #14426 (comment) but heh

@tacaswell tacaswell added this to the v3.2.0 milestone Jun 7, 2019
@tacaswell tacaswell force-pushed the fix_dont_close_on_noop_switch branch from 9bb75fe to 02d3c1b Compare August 19, 2019 13:43
@tacaswell tacaswell force-pushed the fix_dont_close_on_noop_switch branch from b4f9ebb to 3eeb476 Compare August 19, 2019 19:31
dopplershift
dopplershift previously approved these changes Aug 19, 2019
Copy link
Contributor

@dopplershift dopplershift left a comment

Choose a reason for hiding this comment

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

LGTM assuming CI passes.

@tacaswell tacaswell force-pushed the fix_dont_close_on_noop_switch branch from 30ebdf9 to 78ca4cb Compare August 24, 2019 18:17
@tacaswell tacaswell modified the milestones: v3.2.0, v3.3.0 Aug 24, 2019
@tacaswell
Copy link
Member Author

rebased, but there is something funny going on here, pushed to 3.3.

@tacaswell tacaswell force-pushed the fix_dont_close_on_noop_switch branch from 78ca4cb to 5ee99ae Compare January 20, 2020 01:12
@tacaswell tacaswell modified the milestones: v3.3.0, v3.4.0 May 11, 2020
@tacaswell tacaswell force-pushed the fix_dont_close_on_noop_switch branch from 5ee99ae to a7e7cf1 Compare August 24, 2020 02:20
@tacaswell tacaswell dismissed stale reviews from dopplershift and anntzer August 24, 2020 02:20

old

@jklymak jklymak marked this pull request as draft September 10, 2020 15:28
@QuLogic QuLogic modified the milestones: v3.4.0, v3.5.0 Jan 21, 2021
@QuLogic QuLogic modified the milestones: v3.5.0, v3.6.0 Aug 23, 2021
@timhoffm timhoffm modified the milestones: v3.6.0, unassigned Apr 30, 2022
@tacaswell tacaswell modified the milestones: unassigned, v3.7.0 Sep 1, 2022
@tacaswell tacaswell force-pushed the fix_dont_close_on_noop_switch branch from a7e7cf1 to 5ebb5f9 Compare September 1, 2022 22:31
@tacaswell tacaswell marked this pull request as ready for review September 1, 2022 22:33
@timhoffm
Copy link
Member

timhoffm commented Sep 2, 2022

Can we have some documentation on the behavior; maybe in the switch_vackend dockstring?

@timhoffm
Copy link
Member

timhoffm commented Sep 7, 2022

This took us just 6 releases. 😮

@jklymak
Copy link
Member

jklymak commented Oct 23, 2022

@anntzer can you weigh in on this again so Tom doesn't have to rebase any more?

@jklymak jklymak requested a review from anntzer October 23, 2022 20:24
@anntzer
Copy link
Contributor

anntzer commented Oct 23, 2022

I'm still not really convinced it's the right approach (per my comment above), also I'd say that if we really want to go that route we should skip closing whenever the event loops are compatible (e.g. going from qtagg to qtcairo), not just when the backends are exactly equal. OTOH we can always decide on that later and I don't feel overly strongly about this.

@anntzer anntzer merged commit 68c78c9 into matplotlib:main Oct 23, 2022
@tacaswell tacaswell deleted the fix_dont_close_on_noop_switch branch October 23, 2022 23:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants