Skip to content

[Bug]: imsave fails on RGBA data when origin is set to lower #28020

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.

Sign up for GitHub

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

Jump to bottom
Closed
jonas87 opened this issue Apr 4, 2024 · 3 comments · Fixed by #28032
Closed

[Bug]: imsave fails on RGBA data when origin is set to lower #28020

jonas87 opened this issue Apr 4, 2024 · 3 comments · Fixed by #28032
Milestone
v3.9.0

Comments

@jonas87
Copy link

jonas87 commented Apr 4, 2024 •
edited by timhoffm
Loading

Bug summary

Under certain conditions pyplot's imsave() function will fail, with the underlying PIL library throwing an "array is not C-contiguous" error (while the array provided to imsave is C-contiguous).

Code for reproduction

import numpy as np
import matplotlib.pyplot as plt

result = np.zeros((100, 100, 4), dtype='uint8')

print(result.flags) # the ndarray is actually C-contiguous

plt.imsave(fname="test_upper.png", arr=result, format="png", origin="upper")# no problem
plt.imsave(fname="test_lower.png", arr=result, format="png", origin="lower")# error

Actual outcome

File [/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages/matplotlib/pyplot.py:2200](http://localhost:8888/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages/matplotlib/pyplot.py#line=2199), in imsave(fname, arr, **kwargs)
   2198 @_copy_docstring_and_deprecators(matplotlib.image.imsave)
   2199 def imsave(fname, arr, **kwargs):
-> 2200     return matplotlib.image.imsave(fname, arr, **kwargs)

File [/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages/matplotlib/image.py:1659](http://localhost:8888/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages/matplotlib/image.py#line=1658), in imsave(fname, arr, vmin, vmax, cmap, format, origin, dpi, metadata, pil_kwargs)
   1657     pil_kwargs = pil_kwargs.copy()
   1658 pil_shape = (rgba.shape[1], rgba.shape[0])
-> 1659 image = PIL.Image.frombuffer(
   1660     "RGBA", pil_shape, rgba, "raw", "RGBA", 0, 1)
   1661 if format == "png":
   1662     # Only use the metadata kwarg if pnginfo is not set, because the
   1663     # semantics of duplicate keys in pnginfo is unclear.
   1664     if "pnginfo" in pil_kwargs:

File [/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages/PIL/Image.py:3020](http://localhost:8888/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages/PIL/Image.py#line=3019), in frombuffer(mode, size, data, decoder_name, *args)
   3018 if args[0] in _MAPMODES:
   3019     im = new(mode, (1, 1))
-> 3020     im = im._new(core.map_buffer(data, size, decoder_name, 0, args))
   3021     if mode == "P":
   3022         from . import ImagePalette

ValueError: ndarray is not C-contiguous

Expected outcome

saved image

Additional information

  • The input must be an input of size MxNx4. RGBA fails but RGB works.
  • The dtype must be uint8.
  • origin must be set to "lower"
  • Image file type can be set to png, jpg, gif, or tiff, and all trigger the same issue. It's likely not a codec-specific problem.
  • The data in the image does not matter.

Suggestion from stackoverflow user Nick ODell:
[https://github.com/matplotlib/matplotlib/blob/v3.8.3/lib/matplotlib/image.py#L1605](link to code)

If origin == "lower", then the array is reversed in a zero-copy fashion. If this happens, then arr is no longer C contiguous. It then uses ScalarMappable to convert to rgba. However, if the input is already in rgba, it does not copy it. Because of this, using RGB masks the bug, because the copy would be C contiguous.

It then calls PIL.Image.frombuffer, which appears to assume that its input is C contiguous. (Pillow doesn't appear to document this assumption, so this may actually be a Pillow bug?)

Operating system

No response

Matplotlib Version

3.8.3

Matplotlib Backend

No response

Python version

No response

Jupyter version

No response

Installation

None
@ksunden
Copy link
Member

ksunden commented Apr 4, 2024 •
edited
Loading

I will call attention to Pillow's Image.fromarray which calls out:

If obj is not contiguous, then the tobytes method is called and frombuffer() is used.

Which both points to this being intentional on the part of Pillow that frombuffer requires contiguous data (which honestly aligns with the name "buffer", though perhaps could be made more explicit) and suggests that we may be able to fix it by calling the alternate method.

@tacaswell
Copy link
Member

can we pay the copy cost?

@ksunden
Copy link
Member

ksunden commented Apr 4, 2024

To be clear, fromarray is a thin wrapper of frombuffer, which does shape (and dtype, though only if mode is not explicit) validation, calls tobytes if and only if strides != None and calls frombuffer.

So a copy is only made if one is necessary*

* technically, if the the strides given by obj.__array_interface__.get("strides") are the tuple form of C-contiguous strides, rather than the implicit "I'm C Contiguous" marker None, then a copy will be made unnecessarily. By default, numpy arrays do have None in that dictionary when c contiguous from what I see (including views made by reversing an axis and reversing back). but I think it would be valid to have the tuple spelled out.

tacaswell added a commit to tacaswell/matplotlib that referenced this issue Apr 5, 2024
@tacaswell
FIX: ensure images are C order before passing to pillow
6e5d845 
closes matplotlib#28020
@tacaswell tacaswell mentioned this issue Apr 5, 2024
Merged
2 tasks
@tacaswell tacaswell added this to the v3.9.0 milestone Apr 5, 2024
tacaswell added a commit to tacaswell/matplotlib that referenced this issue Apr 15, 2024
@tacaswell
FIX: ensure images are C order before passing to pillow
f8d8020 
closes matplotlib#28020
tacaswell added a commit to tacaswell/matplotlib that referenced this issue Apr 16, 2024
@tacaswell
FIX: ensure images are C order before passing to pillow
df29a4e 
closes matplotlib#28020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v3.9.0
Development

Successfully merging a pull request may close this issue.

FIX: ensure images are C order before passing to pillow tacaswell/matplotlib
3 participants
@tacaswell @ksunden @jonas87