Skip to content

Fix imsave output format when format keyword is None - #10048

Closed
larrybradley wants to merge 3 commits into
matplotlib:masterfrom
larrybradley:fix-imsave-format
Closed

Fix imsave output format when format keyword is None#10048
larrybradley wants to merge 3 commits into
matplotlib:masterfrom
larrybradley:fix-imsave-format

Conversation

@larrybradley

@larrybradley larrybradley commented Dec 19, 2017

Copy link
Copy Markdown
Contributor

PR Summary

Calling imsave without explicitly setting the format keyword (default=None) is supposed to determine the format of the output file from the output filename extension. Currently, if format is not specified (i.e. None), imsave always saves a PNG (despite the output filename):

>>> import imghdr
>>> import matplotlib.pyplot as plt

>>> fn = 'imsave_test.jpg'
>>> plt.imsave(fn, np.zeros((10, 10)))
>>> imghdr.what(fn)    # should return 'jpeg'
'png'

This PR fixes this issue.

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@tacaswell tacaswell added this to the v2.2 milestone Dec 19, 2017
@tacaswell

tacaswell commented Dec 19, 2017

Copy link
Copy Markdown
Member

It looks like appveyor does not have the dependencies to save jpeg

   
        # else report error for unsupported format
        formats = sorted(self.get_supported_filetypes())
        raise ValueError('Format "%s" is not supported.\n'
                         'Supported formats: '
>                        '%s.' % (format, ', '.join(formats)))
E       ValueError: Format "jpg" is not supported.
E       Supported formats: eps, pdf, pgf, png, ps, raw, rgba, svg, svgz.
lib\matplotlib\backend_bases.py:2114: ValueError

Could you either add pillow to what is installed on appveyor or switch to testing svg (you can test that the xml header is in the output file).

@tacaswell

tacaswell commented Dec 19, 2017

Copy link
Copy Markdown
Member

👍 in principle

@larrybradley

larrybradley commented Dec 19, 2017

Copy link
Copy Markdown
Contributor Author

Thanks, @tacaswell. I changed the test to use svg.

tacaswell
tacaswell approved these changes Dec 19, 2017
View reviewed changes

@tacaswell tacaswell left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

modulo tests passing.

@fariza

fariza commented Dec 21, 2017 •
edited
Loading

Copy link
Copy Markdown
Member

@larrybradley travis is failing after calls to imsave

The solution would be to use the same "format check" as it is done inside backend_bases.FigureCanvasBase

if format is None:
            # get format from filename, or from backend's default filetype
            if isinstance(filename, six.string_types):
                format = os.path.splitext(filename)[1][1:]
            if format is None or format == '':
                format = self.get_default_filetype()
                ....

So, that code has to be moved somewhere else as function maybe cbook.__init_py

What about a signature style get_filename_format(filename, format)

@QuLogic QuLogic mentioned this pull request Dec 27, 2017
Closed
6 tasks
@anntzer

anntzer commented Dec 31, 2017

Copy link
Copy Markdown
Contributor

FWIW I would just get rid of the png fast path, which also does not set the dpi value. Basically

diff --git a/lib/matplotlib/image.py b/lib/matplotlib/image.py
index 8b630ce3e..e5924651e 100644
--- a/lib/matplotlib/image.py
+++ b/lib/matplotlib/image.py
@@ -1350,20 +1350,11 @@ def imsave(fname, arr, vmin=None, vmax=None, cmap=None, format=None,
     from matplotlib.backends.backend_agg import FigureCanvasAgg as FigureCanvas
     from matplotlib.figure import Figure
 
-    # Fast path for saving to PNG
-    if (format == 'png' or format is None or
-            isinstance(fname, six.string_types) and
-            fname.lower().endswith('.png')):
-        image = AxesImage(None, cmap=cmap, origin=origin)
-        image.set_data(arr)
-        image.set_clim(vmin, vmax)
-        image.write_png(fname)
-    else:
-        fig = Figure(dpi=dpi, frameon=False)
-        FigureCanvas(fig)
-        fig.figimage(arr, cmap=cmap, vmin=vmin, vmax=vmax, origin=origin,
-                     resize=True)
-        fig.savefig(fname, dpi=dpi, format=format, transparent=True)
+    fig = Figure(dpi=dpi, frameon=False)
+    FigureCanvas(fig)
+    fig.figimage(arr, cmap=cmap, vmin=vmin, vmax=vmax, origin=origin,
+                    resize=True)
+    fig.savefig(fname, dpi=dpi, format=format, transparent=True)
 
 
 def pil_to_array(pilImage):

@fariza

fariza commented Jan 8, 2018

Copy link
Copy Markdown
Member

Why does it exists that Fast path?

@anntzer

anntzer commented Jan 8, 2018

Copy link
Copy Markdown
Contributor

maybe worth timing whether it is actually faster...

@anntzer anntzer mentioned this pull request Jan 11, 2018
Merged
5 of 6 tasks
@anntzer

anntzer commented Jan 12, 2018

Copy link
Copy Markdown
Contributor

Looks like the tests are failing because imsave("foo") (with default rcParams) is now using the "full figure" codepath which gives slightly different results. Probably some tests need to explicitly set the format to png instead.

@tacaswell

tacaswell commented Jan 21, 2018

Copy link
Copy Markdown
Member

Closing this as equivalent changes went in with #10231 .

Thanks for you work on this @larrybradley ! Hope to hear from you again.

@tacaswell tacaswell closed this Jan 21, 2018
@QuLogic QuLogic modified the milestones: needs sorting, v2.2.0 Feb 12, 2018
@larrybradley
larrybradley deleted the fix-imsave-format branch November 13, 2018 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tacaswell tacaswell tacaswell approved these changes

Assignees

No one assigned

Labels

status: duplicate

Projects

None yet

Milestone

v2.2.0

Development

Successfully merging this pull request may close these issues.

5 participants

@larrybradley @tacaswell @fariza @anntzer @QuLogic