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

Improve docstring of imread() and imsave() #11318

Merged
merged 1 commit into from May 28, 2018

Conversation

timhoffm
Copy link
Member

No description provided.

supported via the optional dependency on `Pillow
<http://pillow.readthedocs.io/en/latest/>`_. Note, URL strings
may not be compatible with Pillow. Check the `Pillow documentation
<http://pillow.readthedocs.io/en/latest/>`_ for more information.
Copy link
Contributor

Choose a reason for hiding this comment

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

I would not repeat the link, or, if you really want to do so, use and external hyperlink target (http://docutils.sourceforge.net/docs/user/rst/quickref.html#external-hyperlink-targets). (In fact I would prefer always using external targets through the codebase as they make unrendered versions much more readable IMO, but that's a battle for another day.)

fname : str or file-like
The image file to read. This can be a filename, a URL or a Python
file-like object. If using a file-like object, it must be opened in
binary
Copy link
Contributor

Choose a reason for hiding this comment

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

linewrap

Copy link
Contributor

Choose a reason for hiding this comment

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

would also just write "binary-mode file-like".

-----
Matplotlib can only read PNGs natively. Further image formats are
supported via the optional dependency on Pillow. Note, URL strings
may not be compatible with Pillow. Check the `Pillow documentation`_
Copy link
Contributor

Choose a reason for hiding this comment

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

"are not" (it is easy to check that it's not implemented by Pillow).

----------
fname : str or file-like
The image file to read. This can be a filename, a URL or a Python
file-like object opened in read-binary mode".
Copy link
Contributor

Choose a reason for hiding this comment

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

spurious quote at end

*fname*.
origin : {'upper', 'lower'}, optional
Indicates whether the ``(0, 0)`` index of the array is in the upper
left or lower left corner of the axes. Defaults to :rc:`image.origin`.
Copy link
Member

@jklymak jklymak May 27, 2018

Choose a reason for hiding this comment

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

I find these "Defaults to :rc:blah.bloo" not as helpful as they should be because I don't have the default rcparams memorized. I think we should parenthetically add the default rcParam value in these cases...

Path string to a filename, or a Python file-like object.
If *format* is *None* and *fname* is a string, the output
format is deduced from the extension of the filename.
The filename or a Python file-like object to store the image in.

Choose a reason for hiding this comment

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

I wouldn't kill the info that the filename's extension is used to determine the output format here. People rarely read the complete set of argument docs to find it in the format.

Defaults to :rc:`image.cmap` ('viridis').
format : str, optional
The file format. One of the file extensions supported by the active
backend. Most backends support png, pdf, ps, eps and svg.
Copy link
Contributor

@anntzer anntzer May 28, 2018

Choose a reason for hiding this comment

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

"One of the file extensions supported by the active backend. Most backends support png, pdf, ps, eps and svg."
The first sentence is incorrect (and thus the second unnecessary): the backend will be automatically switched if needed. For example Agg only supports png but you can save pdfs with Agg active, as a temporary pdf canvas will be created on the fly. (And note that the implementation actually always uses an Agg canvas to start with :-))
If you really want to be pedantic the correct list would be "formats either supported by the active backend, or listed in FigureCanvasBase.filetypes", but in practice it's just whatever savefig supports and I wouldn't bother repeating the list here.

@timhoffm timhoffm added this to the v3.0 milestone May 28, 2018
@jklymak jklymak merged commit 2d354ee into matplotlib:master May 28, 2018
@timhoffm timhoffm deleted the doc-imread branch May 28, 2018 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants