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

[Bug]: savefig triggers ValueError with scatter #28123

Open
vegardjervell opened this issue Apr 23, 2024 · 2 comments
Open

[Bug]: savefig triggers ValueError with scatter #28123

vegardjervell opened this issue Apr 23, 2024 · 2 comments

Comments

@vegardjervell
Copy link

vegardjervell commented Apr 23, 2024

Bug summary

Creating a scatterplot with scatter and the linestyle argument (which scatter doesn't actually use), and attempting to save the figure as a pdf with savefig triggers a ValueError.

Code for reproduction

import matplotlib.pyplot as plt
plt.scatter([1, 2], [4, 1], linestyle='') # Need at least two points to trigger error
plt.savefig('my_fig.pdf')

Actual outcome

A pdf file that cannot be opened is produced. The traceback is

Traceback (most recent call last):
  File "/Users/vegardjervell/Documents/sandkasse/mpl_error/demo.py", line 3, in <module>
    plt.savefig('fig.pdf')
  File "/Users/vegardjervell/Documents/sandkasse/mpl_error/venv/lib/python3.11/site-packages/matplotlib/pyplot.py", line 1134, in savefig
    res = fig.savefig(*args, **kwargs)  # type: ignore[func-returns-value]
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/vegardjervell/Documents/sandkasse/mpl_error/venv/lib/python3.11/site-packages/matplotlib/figure.py", line 3390, in savefig
    self.canvas.print_figure(fname, **kwargs)
  File "/Users/vegardjervell/Documents/sandkasse/mpl_error/venv/lib/python3.11/site-packages/matplotlib/backend_bases.py", line 2193, in print_figure
    result = print_method(
             ^^^^^^^^^^^^^
  File "/Users/vegardjervell/Documents/sandkasse/mpl_error/venv/lib/python3.11/site-packages/matplotlib/backend_bases.py", line 2043, in <lambda>
    print_method = functools.wraps(meth)(lambda *args, **kwargs: meth(
                                                                 ^^^^^
  File "/Users/vegardjervell/Documents/sandkasse/mpl_error/venv/lib/python3.11/site-packages/matplotlib/backends/backend_pdf.py", line 2807, in print_pdf
    self.figure.draw(renderer)
  File "/Users/vegardjervell/Documents/sandkasse/mpl_error/venv/lib/python3.11/site-packages/matplotlib/artist.py", line 95, in draw_wrapper
    result = draw(artist, renderer, *args, **kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/vegardjervell/Documents/sandkasse/mpl_error/venv/lib/python3.11/site-packages/matplotlib/artist.py", line 72, in draw_wrapper
    return draw(artist, renderer)
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/vegardjervell/Documents/sandkasse/mpl_error/venv/lib/python3.11/site-packages/matplotlib/figure.py", line 3154, in draw
    mimage._draw_list_compositing_images(
  File "/Users/vegardjervell/Documents/sandkasse/mpl_error/venv/lib/python3.11/site-packages/matplotlib/image.py", line 132, in _draw_list_compositing_images
    a.draw(renderer)
  File "/Users/vegardjervell/Documents/sandkasse/mpl_error/venv/lib/python3.11/site-packages/matplotlib/artist.py", line 72, in draw_wrapper
    return draw(artist, renderer)
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/vegardjervell/Documents/sandkasse/mpl_error/venv/lib/python3.11/site-packages/matplotlib/axes/_base.py", line 3070, in draw
    mimage._draw_list_compositing_images(
  File "/Users/vegardjervell/Documents/sandkasse/mpl_error/venv/lib/python3.11/site-packages/matplotlib/image.py", line 132, in _draw_list_compositing_images
    a.draw(renderer)
  File "/Users/vegardjervell/Documents/sandkasse/mpl_error/venv/lib/python3.11/site-packages/matplotlib/artist.py", line 72, in draw_wrapper
    return draw(artist, renderer)
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/vegardjervell/Documents/sandkasse/mpl_error/venv/lib/python3.11/site-packages/matplotlib/collections.py", line 1005, in draw
    super().draw(renderer)
  File "/Users/vegardjervell/Documents/sandkasse/mpl_error/venv/lib/python3.11/site-packages/matplotlib/artist.py", line 72, in draw_wrapper
    return draw(artist, renderer)
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/vegardjervell/Documents/sandkasse/mpl_error/venv/lib/python3.11/site-packages/matplotlib/collections.py", line 423, in draw
    renderer.draw_path_collection(
  File "/Users/vegardjervell/Documents/sandkasse/mpl_error/venv/lib/python3.11/site-packages/matplotlib/backends/backend_pdf.py", line 2074, in draw_path_collection
    padding = np.max(linewidths)
              ^^^^^^^^^^^^^^^^^^
  File "/Users/vegardjervell/Documents/sandkasse/mpl_error/venv/lib/python3.11/site-packages/numpy/core/fromnumeric.py", line 2810, in max
    return _wrapreduction(a, np.maximum, 'max', axis, None, out,
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/vegardjervell/Documents/sandkasse/mpl_error/venv/lib/python3.11/site-packages/numpy/core/fromnumeric.py", line 88, in _wrapreduction
    return ufunc.reduce(obj, axis, dtype, out, **passkwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ValueError: zero-size array to reduction operation maximum which has no identity

Expected outcome

Because show works fine, and saving the figure as e.g. .png works fine, expected output is the saved figure as a pdf.

That is:

import matplotlib.pyplot as plt
plt.scatter([1, 2], [4, 1], linestyle='')
plt.savefig('my_fig.png') # Works as expected
plt.show() # Works as expected

Additional information

Error appears to be caused by the rendering being incorrectly routed to RendererPdf.draw_path_collection, which assumes there are some linewidths supplied, instead of the renderer that is normally used for scatter.

Expected output is achieved by changing line 2074 in backend_pdf.py to

2074  |  padding = np.max(linewidths) if linewidths else 0

but there is probably a more elegant/robust fix that ensures we route the pdf drawing correctly for a figure without lines, even if the linestyle kwarg is passed to scatter.

Operating system

MacOS 12.5.1

Matplotlib Version

3.8.4

Matplotlib Backend

MacOSX

Python version

3.11.5

Jupyter version

No response

Installation

pip

@ksunden
Copy link
Member

ksunden commented Apr 23, 2024

linestyle is a bit of a strange argument to be passing to scatter. It is not entirely wrong, just may not do what you expect.

If you are expecting a line between your points, then plot is the method to use.

What it is actually controlling, though, is the lines of the markers themselves. This is actually done through **kwargs eventually being passed through the Collection.

Now, given that that is what you want, "" is itself a bit of an edge case, as that is indicated not to draw the line, rather than really adjusting how it is drawn.

In Line2D, the various forms of "linestyle for not having the line" are normalized to "None":

if ls in [' ', '', 'none']:
ls = 'None'

(And then handled within Line2D specific code to not draw anything...)

And In fact if you pass plt.scatter(..., linestyle="None") it does not error, but a line is still drawn...

A quick way to get the desired effect would be to use linewidth=0 instead of adjusting line style. This is actually what is done internally for Patch objects when linestyle is set to "None" (or "", or " ", etc).

Anyway, the Collection set_linestyle mostly wraps mpl.lines._get_dashpattern, just handling the broadcasting since collections are plural:

def set_linestyle(self, ls):
"""
Set the linestyle(s) for the collection.
=========================== =================
linestyle description
=========================== =================
``'-'`` or ``'solid'`` solid line
``'--'`` or ``'dashed'`` dashed line
``'-.'`` or ``'dashdot'`` dash-dotted line
``':'`` or ``'dotted'`` dotted line
=========================== =================
Alternatively a dash tuple of the following form can be provided::
(offset, onoffseq),
where ``onoffseq`` is an even length tuple of on and off ink in points.
Parameters
----------
ls : str or tuple or list thereof
Valid values for individual linestyles include {'-', '--', '-.',
':', '', (offset, on-off-seq)}. See `.Line2D.set_linestyle` for a
complete description.
"""
try:
dashes = [mlines._get_dash_pattern(ls)]
except ValueError:
try:
dashes = [mlines._get_dash_pattern(x) for x in ls]
except ValueError as err:
emsg = f'Do not know how to convert {ls!r} to dashes'
raise ValueError(emsg) from err
# get the list of raw 'unscaled' dash patterns
self._us_linestyles = dashes
# broadcast and scale the lw and dash patterns
self._linewidths, self._linestyles = self._bcast_lwls(
self._us_lw, self._us_linestyles)

The _get_dashpattern function actually returns the same thing for "None" or "solid":

if style in ['solid', 'None']:

But it does not handle all of the aliases ("", " ", etc). And arguably can't... since "not drawing" is different than "what is the offset and dashpattern for the thing I should draw?"

Now, one other wrinkle here is that when given "", the set_linestyle code for Collection will actually result in an empty list (the try section errors, which brings it to the except which loops over the empty string, resulting an an empty list... other aliases will result in an error message at call time):

try:
dashes = [mlines._get_dash_pattern(ls)]
except ValueError:
try:
dashes = [mlines._get_dash_pattern(x) for x in ls]
except ValueError as err:
emsg = f'Do not know how to convert {ls!r} to dashes'
raise ValueError(emsg) from err

The last wrinkle is how it is treated by backends, as agg-based renderers seem to be okay with that? (I think it may actually be failing to draw the collection at all, possibly due to a zip including the empty list? but does not error.) The pdf backend errors

Docs wise, the "None" case, which appears in the Line2D.set_linestyle docstring table is actually left out of the Collection.set_linestyle docstring, though "" does still appear in the parameters section listed, and it crosslinks to the Line2D version as the more complete reference.

Proposed solution:

  • Handle aliasing/special casing of "None" in Collections.set_linestyle
  • Handle interpretting the special case in Collections._bcast_lwls to set lw to 0 for ls="None".

TL;DR:

  • Right now you can use linewidth=0 to get the desired effect
  • In all, internally it is a bit frustrating that everything implements turning off a line via line styling ever so slightly differently, but I think we should likely support it here

@vegardjervell
Copy link
Author

vegardjervell commented Apr 23, 2024

Thank you for the thorough and quick response! I'm aware that it really doesn't make sense to pass a linestyle to scatter, but I often use plot(*args, linestyle='') if I'm scattering many points with the same color, as scatter can be quite a bit slower if there are many points (at least I remember that being the case some years ago, please correct me if I'm wrong). The reason I came across this was that I modified some code to set different colors on the points, and forgot to remove the linestyle kwarg (which I think may be not-so-uncommon to do).

The reason I think it qualifies as a bug is maybe more the cryptic error message that is thrown, specifically when saving as a pdf, when saving as png and show work as expected.

I think it would be a perfectly reasonable fix to just throw an error if linestyle is passed to scatter, but given that this can be handled elegantly in Collection.set_linestyle or Collection._bcast_lwls that would of course be preferable.

vegardjervell added a commit to vegardjervell/matplotlib that referenced this issue Apr 24, 2024
Make Collection.set_linestyle normalize the empty string to 'None', and make Collection._bcast_lwls force linewidths to 0 if the dashes on_off_seq is None.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants