8000 Add warning for multiple pyplot.figure calls with same ID by Impaler343 · Pull Request #27992 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]
Skip to content

Add warning for multiple pyplot.figure calls with same ID #27992

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.

Sign up for GitHub

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 5 commits into from
May 19, 2024
Merged

Add warning for multiple pyplot.figure calls with same ID #27992

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d41b048
Add warning for multiple pyplot.figure calls with same ID
Impaler343 Mar 29, 2024
a9e9723
Added test for warning
Impaler343 May 8, 2024
18eb3e7
Added test for uncovered line
Impaler343 May 13, 2024
dd9cda0
Added warning for recurrent mention using figure object and tests
Impaler343 May 19, 2024
af11852
Minor Fix
Impaler343 May 19, 2024
File filter

Filter by extension

Filter by extension
Conversations < 8000 include-fragment data-nonce="v2:ddaa82fe-c89b-dec0-8666-ecc94f6083c1" data-view-component="true">
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 24 additions & 12 deletions lib/matplotlib/pyplot.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -983,30 +983,42 @@ def figure(
`~matplotlib.rcParams` defines the default values, which can be modified
in the matplotlibrc file.
"""
allnums = get_fignums()

if isinstance(num, FigureBase):
# type narrowed to `Figure | SubFigure` by combination of input and isinstance
if num.canvas.manager is None:
raise ValueError("The passed figure is not managed by pyplot")
elif any([figsize, dpi, facecolor, edgecolor, not frameon,
kwargs]) and num.canvas.manager.num in allnums:
_api.warn_external(
"Ignoring specified arguments in this call "
f"because figure with num: {num.canvas.manager.num} already exists")
_pylab_helpers.Gcf.set_active(num.canvas.manager)
return num.figure

allnums = get_fignums()
next_num = max(allnums) + 1 if allnums else 1
fig_label = ''
if num is None:
num = next_num
elif isinstance(num, str):
fig_label = num
all_labels = get_figlabels()
if fig_label not in all_labels:
if fig_label == 'all':
_api.warn_external("close('all') closes all existing figures.")
num = next_num
else:
inum = all_labels.index(fig_label)
num = allnums[inum]
else:
num = int(num) # crude validation of num argument
if any([figsize, dpi, facecolor, edgecolor, not frameon,
kwargs]) and num in allnums:
Copy link
Member

Choose a reason for hiding this comment

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

Reading this one more time, I think we also need the warning around line 988 for the case were someone does:

fig = plt.figure()
plt.figure(fig, figsize=(2, 2))  # this should warn as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, added that as well

_api.warn_external(
"Ignoring specified arguments in this call "
f"because figure with num: {num} already exists")
if isinstance(num, str):
fig_label = num
all_labels = get_figlabels()
if fig_label not in all_labels:
if fig_label == 'all':
_api.warn_external("close('all') closes all existing figures.")
num = next_num
else:
inum = all_labels.index(fig_label)
num = allnums[inum]
else:
num = int(num) # crude validation of num argument

# Type of "num" has narrowed to int, but mypy can't quite see it
manager = _pylab_helpers.Gcf.get_fig_manager(num) # type: ignore[arg-type]
Expand Down
19 changes: 19 additions & 0 deletions lib/matplotlib/tests/test_pyplot.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -457,3 +457,22 @@ def test_figure_hook():
fig = plt.figure()

assert fig._test_was_here


def test_multiple_same_figure_calls():
fig = mpl.pyplot.figure(1, figsize=(1, 2))
with pytest.warns(UserWarning, match="Ignoring specified arguments in this call"):
fig2 = mpl.pyplot.figure(1, figsize=(3, 4))
with pytest.warns(UserWarning, match="Ignoring specified arguments in this call"):
mpl.pyplot.figure(fig, figsize=(5, 6))
assert fig is fig2
fig3 = mpl.pyplot.figure(1) # Checks for false warnings
assert fig is fig3


def test_close_all_warning():
fig1 = plt.figure()

# Check that the warning is issued when 'all' is passed to plt.figure
with pytest.warns(UserWarning, match="closes all existing figures"):
fig2 = plt.figure("all")
Loading
0