Skip to content

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

Merged
anntzer merged 3 commits intomatplotlib:mainfrom
tacaswell:fix_dont_close_on_noop_switch
Oct 23, 2022
Merged

FIX: don't close figures if switch_backend is a no-op#14471
anntzer merged 3 commits intomatplotlib:mainfrom
tacaswell:fix_dont_close_on_noop_switch

Conversation

@tacaswell
Copy link
Copy Markdown
Member

@tacaswell tacaswell commented Jun 6, 2019

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

@tacaswell tacaswell mentioned this pull request Jun 6, 2019
Closed
anntzer
anntzer reviewed Jun 6, 2019
View reviewed changes
anntzer
anntzer previously approved these changes Jun 6, 2019
View reviewed changes
Copy link
Copy Markdown
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
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
Member Author

tacaswell commented Aug 24, 2019

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

This was referenced Dec 12, 2019
Closed
Closed
@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
Copy Markdown
Member

timhoffm commented Sep 2, 2022

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

@timhoffm
Copy link
Copy Markdown
Member

timhoffm commented Sep 7, 2022

This took us just 6 releases. 😮

@jklymak
Copy link
Copy Markdown
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
Copy Markdown
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
@anntzer anntzer mentioned this pull request Aug 8, 2023
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@anntzer anntzer anntzer approved these changes

@timhoffm timhoffm timhoffm approved these changes

@dopplershift dopplershift dopplershift left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v3.7.0

Development

Successfully merging this pull request may close these issues.

6 participants

@tacaswell @timhoffm @jklymak @anntzer @dopplershift @QuLogic