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

Return filename from save_figure #27766

Merged
merged 21 commits into from
Oct 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions doc/api/next_api_changes/behavior/27744-FM.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
``NavigationToolbar2.save_figure`` now returns filepath of saved figure
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

``NavigationToolbar2.save_figure`` function may return the filename of the saved figure.

If a backend implements this functionality it should return `None`
in the case where no figure is actually saved (because the user closed the dialog without saving).

If the backend does not or can not implement this functionality (currently the Gtk4 backends
and webagg backends do not) this method will return ``NavigationToolbar2.UNKNOWN_SAVED_STATUS``.
23 changes: 22 additions & 1 deletion lib/matplotlib/backend_bases.py
Original file line number Diff line number Diff line change
Expand Up @@ -2858,6 +2858,8 @@
('Save', 'Save the figure', 'filesave', 'save_figure'),
)

UNKNOWN_SAVED_STATUS = object()

def __init__(self, canvas):
self.canvas = canvas
canvas.toolbar = self
Expand Down Expand Up @@ -3272,7 +3274,26 @@
return self.subplot_tool

def save_figure(self, *args):
"""Save the current figure."""
"""
Save the current figure.

Backend implementations may choose to return
the absolute path of the saved file, if any, as
a string.

If no file is created then `None` is returned.

If the backend does not implement this functionality
then `NavigationToolbar2.UNKNOWN_SAVED_STATUS` is returned.

Check warning on line 3287 in lib/matplotlib/backend_bases.py

View check run for this annotation

Codecov / codecov/patch

lib/matplotlib/backend_bases.py#L3287

Added line #L3287 was not covered by tests
Copy link
Member

Choose a reason for hiding this comment

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

Repeating my self a bit from the main comments, the problem is that backends that don't implement any of this still return None.


Returns
-------
str or `NavigationToolbar2.UNKNOWN_SAVED_STATUS` or `None`
The filepath of the saved figure.

Check warning on line 3292 in lib/matplotlib/backend_bases.py

View check run for this annotation

Codecov / codecov/patch

lib/matplotlib/backend_bases.py#L3292

Added line #L3292 was not covered by tests
Returns `None` if figure is not saved.
Returns `NavigationToolbar2.UNKNOWN_SAVED_STATUS` when
the backend does not provide the information.
"""
raise NotImplementedError

def update(self):
Expand Down
3 changes: 2 additions & 1 deletion lib/matplotlib/backend_bases.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,7 @@ class _Mode(str, Enum):

class NavigationToolbar2:
toolitems: tuple[tuple[str, ...] | tuple[None, ...], ...]
UNKNOWN_SAVED_STATUS: object
canvas: FigureCanvasBase
mode: _Mode
def __init__(self, canvas: FigureCanvasBase) -> None: ...
Expand Down Expand Up @@ -441,7 +442,7 @@ class NavigationToolbar2:
def push_current(self) -> None: ...
subplot_tool: widgets.SubplotTool
def configure_subplots(self, *args): ...
def save_figure(self, *args) -> None: ...
def save_figure(self, *args) -> str | None | object: ...
def update(self) -> None: ...
def set_history_buttons(self) -> None: ...

Expand Down
3 changes: 2 additions & 1 deletion lib/matplotlib/backends/_backend_tk.py
Original file line number Diff line number Diff line change
Expand Up @@ -867,7 +867,7 @@ def save_figure(self, *args):
)

if fname in ["", ()]:
return
return None
# Save dir for next time, unless empty str (i.e., use cwd).
if initialdir != "":
mpl.rcParams['savefig.directory'] = (
Expand All @@ -882,6 +882,7 @@ def save_figure(self, *args):

try:
self.canvas.figure.savefig(fname, format=extension)
return fname
except Exception as e:
tkinter.messagebox.showerror("Error saving file", str(e))

Expand Down
9 changes: 5 additions & 4 deletions lib/matplotlib/backends/backend_gtk3.py
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@
def save_figure(self, *args):
dialog = Gtk.FileChooserDialog(
title="Save the figure",
parent=self.canvas.get_toplevel(),
transient_for=self.canvas.get_toplevel(),
action=Gtk.FileChooserAction.SAVE,
buttons=(Gtk.STOCK_CANCEL, Gtk.ResponseType.CANCEL,
Gtk.STOCK_SAVE, Gtk.ResponseType.OK),
Expand Down Expand Up @@ -371,16 +371,17 @@
fmt = self.canvas.get_supported_filetypes_grouped()[ff.get_name()][0]
dialog.destroy()
if response != Gtk.ResponseType.OK:
return
return None

Check warning on line 374 in lib/matplotlib/backends/backend_gtk3.py

View check run for this annotation

Codecov / codecov/patch

lib/matplotlib/backends/backend_gtk3.py#L374

Added line #L374 was not covered by tests
# Save dir for next time, unless empty str (which means use cwd).
if mpl.rcParams['savefig.directory']:
mpl.rcParams['savefig.directory'] = os.path.dirname(fname)
try:
self.canvas.figure.savefig(fname, format=fmt)
return fname
except Exception as e:
dialog = Gtk.MessageDialog(
parent=self.canvas.get_toplevel(), message_format=str(e),
type=Gtk.MessageType.ERROR, buttons=Gtk.ButtonsType.OK)
transient_for=self.canvas.get_toplevel(), text=str(e),
message_type=Gtk.MessageType.ERROR, buttons=Gtk.ButtonsType.OK)
dialog.run()
dialog.destroy()

Expand Down
1 change: 1 addition & 0 deletions lib/matplotlib/backends/backend_gtk4.py
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,7 @@
msg.show()

dialog.show()
return self.UNKNOWN_SAVED_STATUS

Check warning on line 394 in lib/matplotlib/backends/backend_gtk4.py

View check run for this annotation

Codecov / codecov/patch

lib/matplotlib/backends/backend_gtk4.py#L394

Added line #L394 was not covered by tests


class ToolbarGTK4(ToolContainerBase, Gtk.Box):
Expand Down
1 change: 1 addition & 0 deletions lib/matplotlib/backends/backend_macosx.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@
if mpl.rcParams['savefig.directory']:
mpl.rcParams['savefig.directory'] = os.path.dirname(filename)
self.canvas.figure.savefig(filename)
return filename

Check warning on line 145 in lib/matplotlib/backends/backend_macosx.py

View check run for this annotation

Codecov / codecov/patch

lib/matplotlib/backends/backend_macosx.py#L145

Added line #L145 was not covered by tests


class FigureManagerMac(_macosx.FigureManager, FigureManagerBase):
Expand Down
1 change: 1 addition & 0 deletions lib/matplotlib/backends/backend_qt.py
Original file line number Diff line number Diff line change
Expand Up @@ -839,6 +839,7 @@ def save_figure(self, *args):
self, "Error saving file", str(e),
QtWidgets.QMessageBox.StandardButton.Ok,
QtWidgets.QMessageBox.StandardButton.NoButton)
return fname

def set_history_buttons(self):
can_backward = self._nav_stack._pos > 0
Expand Down
3 changes: 2 additions & 1 deletion lib/matplotlib/backends/backend_webagg_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -403,8 +403,9 @@
self.canvas.send_event("rubberband", x0=-1, y0=-1, x1=-1, y1=-1)

def save_figure(self, *args):
"""Save the current figure"""
"""Save the current figure."""
self.canvas.send_event('save')
return self.UNKNOWN_SAVED_STATUS

Check warning on line 408 in lib/matplotlib/backends/backend_webagg_core.py

View check run for this annotation

Codecov / codecov/patch

lib/matplotlib/backends/backend_webagg_core.py#L408

Added line #L408 was not covered by tests

def pan(self):
super().pan()
Expand Down
1 change: 1 addition & 0 deletions lib/matplotlib/backends/backend_wx.py
Original file line number Diff line number Diff line change
Expand Up @@ -1143,6 +1143,7 @@
mpl.rcParams["savefig.directory"] = str(path.parent)
try:
self.canvas.figure.savefig(path, format=fmt)
return path

Check warning on line 1146 in lib/matplotlib/backends/backend_wx.py

View check run for this annotation

Codecov / codecov/patch

lib/matplotlib/backends/backend_wx.py#L1146

Added line #L1146 was not covered by tests
except Exception as e:
dialog = wx.MessageDialog(
parent=self.canvas.GetParent(), message=str(e),
Expand Down
26 changes: 26 additions & 0 deletions lib/matplotlib/tests/test_backend_gtk3.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import os
from matplotlib import pyplot as plt

import pytest
from unittest import mock


@pytest.mark.backend("gtk3agg", skip_on_importerror=True)
Expand Down Expand Up @@ -46,3 +48,27 @@ def receive(event):
fig.canvas.mpl_connect("draw_event", send)
fig.canvas.mpl_connect("key_press_event", receive)
plt.show()


@pytest.mark.backend("gtk3agg", skip_on_importerror=True)
def test_save_figure_return():
from gi.repository import Gtk
fig, ax = plt.subplots()
ax.imshow([[1]])
with mock.patch("gi.repository.Gtk.FileFilter") as fileFilter:
filt = fileFilter.return_value
filt.get_name.return_value = "Portable Network Graphics"
with mock.patch("gi.repository.Gtk.FileChooserDialog") as dialogChooser:
dialog = dialogChooser.return_value
dialog.get_filter.return_value = filt
dialog.get_filename.return_value = "foobar.png"
dialog.run.return_value = Gtk.ResponseType.OK
fname = fig.canvas.manager.toolbar.save_figure()
os.remove("foobar.png")
assert fname == "foobar.png"

with mock.patch("gi.repository.Gtk.MessageDialog"):
dialog.get_filename.return_value = None
dialog.run.return_value = Gtk.ResponseType.OK
fname = fig.canvas.manager.toolbar.save_figure()
assert fname is None
15 changes: 15 additions & 0 deletions lib/matplotlib/tests/test_backend_macosx.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import os

import pytest
from unittest import mock

import matplotlib as mpl
import matplotlib.pyplot as plt
Expand Down Expand Up @@ -50,3 +51,17 @@
def test_ipython():
from matplotlib.testing import ipython_in_subprocess
ipython_in_subprocess("osx", {(8, 24): "macosx", (7, 0): "MacOSX"})


@pytest.mark.backend('macosx')

Check warning on line 56 in lib/matplotlib/tests/test_backend_macosx.py

View check run for this annotation

Codecov / codecov/patch

lib/matplotlib/tests/test_backend_macosx.py#L56

Added line #L56 was not covered by tests
def test_save_figure_return():
fig, ax = plt.subplots()
ax.imshow([[1]])
prop = "matplotlib.backends._macosx.choose_save_file"

Check warning on line 60 in lib/matplotlib/tests/test_backend_macosx.py

View check run for this annotation

Codecov / codecov/patch

lib/matplotlib/tests/test_backend_macosx.py#L58-L60

Added lines #L58 - L60 were not covered by tests
with mock.patch(prop, return_value="foobar.png"):
fname = fig.canvas.manager.toolbar.save_figure()
os.remove("foobar.png")
assert fname == "foobar.png"

Check warning on line 64 in lib/matplotlib/tests/test_backend_macosx.py

View check run for this annotation

Codecov / codecov/patch

lib/matplotlib/tests/test_backend_macosx.py#L62-L64

Added lines #L62 - L64 were not covered by tests
with mock.patch(prop, return_value=None):
fname = fig.canvas.manager.toolbar.save_figure()
assert fname is None

Check warning on line 67 in lib/matplotlib/tests/test_backend_macosx.py

View check run for this annotation

Codecov / codecov/patch

lib/matplotlib/tests/test_backend_macosx.py#L66-L67

Added lines #L66 - L67 were not covered by tests
14 changes: 14 additions & 0 deletions lib/matplotlib/tests/test_backend_qt.py
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,20 @@
fig.canvas.manager.toolbar.edit_parameters()


@pytest.mark.backend('QtAgg', skip_on_importerror=True)
def test_save_figure_return():
fig, ax = plt.subplots()
ax.imshow([[1]])
prop = "matplotlib.backends.qt_compat.QtWidgets.QFileDialog.getSaveFileName"
with mock.patch(prop, return_value=("foobar.png", None)):
fname = fig.canvas.manager.toolbar.save_figure()
os.remove("foobar.png")
assert fname == "foobar.png"
with mock.patch(prop, return_value=(None, None)):
fname = fig.canvas.manager.toolbar.save_figure()
assert fname is None

Check warning on line 240 in lib/matplotlib/tests/test_backend_qt.py

View check run for this annotation

Codecov / codecov/patch

lib/matplotlib/tests/test_backend_qt.py#L240

Added line #L240 was not covered by tests

@pytest.mark.backend('QtAgg', skip_on_importerror=True)
def test_figureoptions_with_datetime_axes():
fig, ax = plt.subplots()
Expand Down
17 changes: 17 additions & 0 deletions lib/matplotlib/tests/test_backend_tk.py
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,23 @@ class Toolbar(NavigationToolbar2Tk):
print("success")


@_isolated_tk_test(success_count=2)
def test_save_figure_return():
import matplotlib.pyplot as plt
from unittest import mock
fig = plt.figure()
prop = "tkinter.filedialog.asksaveasfilename"
with mock.patch(prop, return_value="foobar.png"):
fname = fig.canvas.manager.toolbar.save_figure()
os.remove("foobar.png")
assert fname == "foobar.png"
print("success")
with mock.patch(prop, return_value=""):
fname = fig.canvas.manager.toolbar.save_figure()
assert fname is None
print("success")


@_isolated_tk_test(success_count=1)
def test_canvas_focus():
import tkinter as tk
Expand Down
Loading