Skip to content

[Bug]: String contour(colors) gives confusing error when extend used #28782

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

Closed
lucyleeow opened this issue Sep 5, 2024 · 7 comments · Fixed by #28786
Closed

[Bug]: String contour(colors) gives confusing error when extend used #28782

lucyleeow opened this issue Sep 5, 2024 · 7 comments · Fixed by #28786

Comments

@lucyleeow
Copy link
Contributor

lucyleeow commented Sep 5, 2024

Bug summary

contour (and friends) specify that a single string can be passed to colors:

As a shortcut, single color strings may be used in place of one-element lists

But this only works when levels is the same length as the color string.

Code for reproduction

import numpy as np
import matplotlib.pyplot as plt

x = np.arange(1, 10)
y = x.reshape(-1, 1)
h = x * y

cs = plt.contour(h, levels=[10, 30, 50],
    colors='green', extend='both')

Actual outcome

colors='green' gives error (while colors='red' works, or making levels to be length of 5 also works):

---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
Cell In[64], line 8
      5 y = x.reshape(-1, 1)
      6 h = x * y
----> 8 cs = plt.contour(h, levels=[10, 30, 50],
      9     colors='green', extend='both')

File ~/miniconda3/envs/sklearn-env/lib/python3.12/site-packages/matplotlib/pyplot.py:3143, in contour(data, *args, **kwargs)
   3141 @_copy_docstring_and_deprecators(Axes.contour)
   3142 def contour(*args, data=None, **kwargs) -> QuadContourSet:
-> 3143     __ret = gca().contour(
   3144         *args, **({"data": data} if data is not None else {}), **kwargs
   3145     )
   3146     if __ret._A is not None:  # type: ignore[attr-defined]
   3147         sci(__ret)

File ~/miniconda3/envs/sklearn-env/lib/python3.12/site-packages/matplotlib/__init__.py:1473, in _preprocess_data.<locals>.inner(ax, data, *args, **kwargs)
   1470 @functools.wraps(func)
   1471 def inner(ax, *args, data=None, **kwargs):
   1472     if data is None:
-> 1473         return func(
   1474             ax,
   1475             *map(sanitize_sequence, args),
   1476             **{k: sanitize_sequence(v) for k, v in kwargs.items()})
   1478     bound = new_sig.bind(ax, *args, **kwargs)
   1479     auto_label = (bound.arguments.get(label_namer)
   1480                   or bound.kwargs.get(label_namer))

File ~/miniconda3/envs/sklearn-env/lib/python3.12/site-packages/matplotlib/axes/_axes.py:6659, in Axes.contour(self, *args, **kwargs)
   6650 """
   6651 Plot contour lines.
   6652 
   (...)
   6656 %(contour_doc)s
   6657 """
   6658 kwargs['filled'] = False
-> 6659 contours = mcontour.QuadContourSet(self, *args, **kwargs)
   6660 self._request_autoscale_view()
   6661 return contours

File ~/miniconda3/envs/sklearn-env/lib/python3.12/site-packages/matplotlib/contour.py:847, in ContourSet.__init__(self, ax, levels, filled, linewidths, linestyles, hatches, alpha, origin, extent, cmap, colors, norm, vmin, vmax, extend, antialiased, nchunk, locator, transform, negative_linestyles, clip_path, *args, **kwargs)
    845             cmap.set_under(self.colors[0])
    846         if self._extend_max:
--> 847             cmap.set_over(self.colors[-1])
    849 # label lists must be initialized here
    850 self.labelTexts = []

File ~/miniconda3/envs/sklearn-env/lib/python3.12/site-packages/matplotlib/colors.py:834, in Colormap.set_over(self, color, alpha)
    832 def set_over(self, color='k', alpha=None):
    833     """Set the color for high out-of-range values."""
--> 834     self._rgba_over = to_rgba(color, alpha)
    835     if self._isinit:
    836         self._set_extremes()

File ~/miniconda3/envs/sklearn-env/lib/python3.12/site-packages/matplotlib/colors.py:314, in to_rgba(c, alpha)
    312     rgba = None
    313 if rgba is None:  # Suppress exception chaining of cache lookup failure.
--> 314     rgba = _to_rgba_no_colorcycle(c, alpha)
    315     try:
    316         _colors_full_map.cache[c, alpha] = rgba

File ~/miniconda3/envs/sklearn-env/lib/python3.12/site-packages/matplotlib/colors.py:391, in _to_rgba_no_colorcycle(c, alpha)
    387             raise ValueError(
    388                 f"Invalid string grayscale value {orig_c!r}. "
    389                 f"Value must be within 0-1 range")
    390         return c, c, c, alpha if alpha is not None else 1.
--> 391     raise ValueError(f"Invalid RGBA argument: {orig_c!r}")
    392 # turn 2-D array into 1-D array
    393 if isinstance(c, np.ndarray):

ValueError: Invalid RGBA argument: 'n'

Expected outcome

Does not depend on color string length.

Additional information

I think it is because we don't convert to list if colors is given as string, and then we do this:

if (len(self.colors) == total_levels and
(self._extend_min or self._extend_max)):

Operating system

Ubuntu

Matplotlib Version

3.9.2

Matplotlib Backend

No response

Python version

No response

Jupyter version

No response

Installation

None

@rcomer
Copy link
Member

rcomer commented Sep 5, 2024

Thanks for the report @lucyleeow! I have reproduced this with our main development branch and back to v3.6.

@rcomer
Copy link
Member

rcomer commented Sep 5, 2024

Actually, now I look again, this seems to only be an issue if using contour with the extend keyword. If I omit extend, all is well. If I use contourf instead, the plot works (although a completely green axes is not very useful).

extend is not meaningful with contour, so I think we should either raise a more informative error here, or have contour throw away this parameter.

@lucyleeow lucyleeow changed the title [Bug]: Using string contour(colors) only works if levels same length as string [Bug]: String contour(colors) gives confusing error when extend used Sep 6, 2024
@lucyleeow
Copy link
Contributor Author

lucyleeow commented Sep 6, 2024

Ah yes you're right. I think I was confused because it seems that contour and contourf seem to share the same parameters in the documentation, and I just copied the example from the contour page which happened to be for extend and use contourf.

Maybe contour could ignore extend but you give a warning to tell users that contour does not take extend (adn was thus ignored)?
Some improvement to the docs would also be nice, maybe you could make it clearer that extend is only for contourf (like what has been done for the linewidths parameter, which specifies in italics on the first line that it only applies to contour?). I think ideally contour and contourf would only include parameters relevant to each function in the docs but I can understand that this may be too difficult to achieve.

@jklymak
Copy link
Member

jklymak commented Sep 6, 2024

It's pretty hard to see what these two things have to do with each other. We definitely should clean this up

@rcomer
Copy link
Member

rcomer commented Sep 6, 2024

I have now learned that we can't just ignore extend for contour because that has implications for colorbars. Specifically, I broke two image tests.

https://github.com/matplotlib/matplotlib/blob/main/lib/matplotlib/tests/baseline_images/test_colorbar/contour_colorbar.png
https://github.com/matplotlib/matplotlib/blob/main/lib/matplotlib/tests/baseline_images/test_contour/contour_manual_colors_and_levels.png

So I guess we do need to fix single-colour support?

@jklymak
Copy link
Member

jklymak commented Sep 6, 2024

That is a nutty use of a colorbar, but I think we do need to keep supporting it.

Isn't it the case that a single string will always be a colorspec? We aren't going to allow a spec like 'rgbyk' to represent three five colors are we? Or is that what it did before? If so, that is a terrible API.

@timhoffm
Copy link
Member

timhoffm commented Sep 6, 2024

I agree: 'rgbyk' should not be a valid color spec.

@QuLogic QuLogic added this to the v3.10.0 milestone Sep 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants