-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Add note on writing binary formats to stdout using savefig() #18068
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
Conversation
lib/matplotlib/figure.py
Outdated
@@ -2199,6 +2199,10 @@ def savefig(self, fname, *, transparent=None, **kwargs): | |||
:rc:`savefig.format` and the appropriate extension is appended to | |||
*fname*. | |||
|
|||
To write a binary format like PNG to ``sys.stdout``, use the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems pretty in the weeds to me and adds a technical detail to an already long docstring. I really think the user who wanted to write a png to standard out just didn't realize that a png is not a text format, because I can't see what the use is in dumping the contents to stdout.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could write a program that outputs the binary image and pipe the output to some other program, e.g. python png_stdout.py | convert -trim - result.png
.
Maybe not the most common use case, but valid nevertheless.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd say thats advanced enough usage that we can assume whoever may want to do that understands how to do the pipe on the python end. I disagree that our docstring should document a python feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a documentation problem on our side: We claim to support file-like
objects. sys.stdout
is one, but it does not work.
Alternatively we'd have to do something like:
fname : str or path-like or binary file-like
which is really terse and possibly not understandable unless you're an expert and already know about https://docs.python.org/3/library/sys.html#sys.stdout
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I won't block. I just disagree that we need to specify what "file-like" means in the context of stdout
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will binary file-like work with svg/eps/etc?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, all formats will work with a binary file-like.
Since matplotlib is written in python, having to document python feature(s) seems reasonable... |
That assumption would be wrong in my case, or I'd first assume a bug in matplotlib and open referenced issue |
This is the same error as the one you get when doing e.g. |
I'm +0 this. It is a useful trick to know, but maybe not worth putting in the savefig docstring? I would definitely merge an example showing this. Either way do we need something saying that the file-like needs to be in binary mode? While it is true that Matplotlib is written in Python, that does not mean we should fully document Python in our docs. |
I think the savefig docstring should replace An example in the examples directory is fine, I guess. |
ef0d7cf
to
5a4cd6f
Compare
Let's go with the minimal |
Self-merging based on the positive review. |
PR Summary
Document how to handle #18049.