Skip to content
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

Remove unused 'close' arg to show(). #466

Merged
merged 1 commit into from
Jun 2, 2022
Merged

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented May 22, 2022

For reference, _Backend_ipympl.show is effectively the function that
pyplot.show calls (pyplot.show even copies the signature of the backend
show).

In af699c2, a 'close' kwarg was added to _Backend_ipympl.show, although
any use of it was immediately removed in a subsequent commit (783419b).
Still, this change means that calling e.g. plt.show(True), for
example, would no longer set block=True, but instead set close=True.
As the change is relatively recent, it seems better to remove the unused
'close' kwarg again, which also makes the signature consistent with the
builtin Matplotlib backends.

For reference, _Backend_ipympl.show is effectively the function that
pyplot.show calls (pyplot.show even copies the signature of the backend
show).

In af699c2, a 'close' kwarg was added to _Backend_ipympl.show, although
any use of it was immediately removed in a subsequent commit (783419b).
Still, this change means that calling e.g. `plt.show(True)`, for
example, would no longer set `block=True`, but instead set `close=True`.
As the change is relatively recent, it seems better to remove the unused
'close' kwarg again, which also makes the signature consistent with the
builtin Matplotlib backends.
@github-actions
Copy link
Contributor

Binder 👈 Launch a binder notebook on branch anntzer/ipympl/unclose

@ianhi
Copy link
Collaborator

ianhi commented May 23, 2022

Makes sense to me. Thanks for updating here as you go on the main repo :)

Only concern is the visual regression test failing. Maybe arestart will help

@ianhi ianhi closed this May 23, 2022
@ianhi ianhi reopened this May 23, 2022
@anntzer
Copy link
Contributor Author

anntzer commented Jun 1, 2022

@ianhi AFAICT the same visual regression test also failed previously with https://github.com/matplotlib/ipympl/actions/runs/2333504817?

Copy link
Collaborator

@ianhi ianhi left a comment

Choose a reason for hiding this comment

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

ah indeed good catch! thanks again!

@ianhi ianhi merged commit 913431e into matplotlib:main Jun 2, 2022
@anntzer anntzer deleted the unclose branch June 2, 2022 23:35
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.

2 participants