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

Open
wants to merge 21 commits into
base: main
Choose a base branch
from
Open

Return filename from save_figure #27766

wants to merge 21 commits into from

Conversation

Zybulon
Copy link

@Zybulon Zybulon commented Feb 10, 2024

Closes #27744

PR summary

This PR adresses issue #27744.
save_figure functions from the NavigationToolbar return the filename of the saved figure.
If no figure is save then it returns None.
For GTK4 backend and Web backend I could not get the filename so the function still returns None.

PR checklist

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Thank you for opening your first PR into Matplotlib!

If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. Most of our reviewers are volunteers and sometimes things fall through the cracks.

You can also join us on gitter for real-time discussion.

For details on testing, writing docs, and our review process, please see the developer guide

We strive to be a welcoming and open project. Please follow our Code of Conduct.

@Zybulon Zybulon marked this pull request as ready for review February 10, 2024 16:16
@tacaswell tacaswell added this to the v3.9.0 milestone Feb 10, 2024
@tacaswell
Copy link
Member

We can probably use a closure / nonlocal with the gtk response callback, but I'm not sure if dialog.show() blocks until the window is closed or return immediately. If it blocks then we can get the the filename out of that one too.

I agree we the webagg one looks like a big lift because I don't think we currently have a response back over the websocket. Even if we added that, it is not super clear it would be useful as the filesystems that the client (in a browser possibly on a different machine) is in general not the filesystem that the server sees. There is also a good case we should not leak anything about the client file system back to the server.

@tacaswell
Copy link
Member

If we want to differentiate between "I did not save a file" and "I can not tell you if a file was saved" we could return a different sentinel in the "I did not save a file case". The simplest thing (from an implement ion point of view...typing this proposal would be hard) would be to add NO_FILE_SAVED = object() as a class attribute on NavigationToolbar2 and return that if no file is saved.

@Zybulon
Copy link
Author

Zybulon commented Feb 10, 2024

That is a good idea. I'll do that.

@Zybulon
Copy link
Author

Zybulon commented Feb 10, 2024

Usually the FileDialogs (from Qt, Gtk etc...) return None when no file is selected. To be consistent with this behavior, I propose returning None when no file is saved and a class attribute UNKNOWN_SAVED_STATUS = object() when the status is unknown. What do you think ?

@Zybulon Zybulon marked this pull request as draft February 11, 2024 09:15
@Zybulon Zybulon force-pushed the main branch 3 times, most recently from 64b6642 to 20c4e55 Compare May 20, 2024 06:59
@Zybulon Zybulon marked this pull request as draft May 20, 2024 16:22
@Zybulon Zybulon marked this pull request as ready for review May 20, 2024 16:23
@Zybulon
Copy link
Author

Zybulon commented May 20, 2024

Hello,
Sorry for the delay. I think the pull request is ready for review when someone has time to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

[ENH]: save_figure returns image fullpath
5 participants