Skip to content

[MNT]: ListedColormap inconsistencies #28763

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
oscargus opened this issue Aug 29, 2024 · 5 comments
Closed

[MNT]: ListedColormap inconsistencies #28763

oscargus opened this issue Aug 29, 2024 · 5 comments
Labels
API: consistency Documentation: API files in lib/ and doc/api Maintenance topic: color/color & colormaps
Milestone
v3.11.0

Comments

@oscargus
Copy link
Member

oscargus commented Aug 29, 2024 •
edited
Loading

Summary

The documentation to ListedColormap states that the input should be a list or array

matplotlib/lib/matplotlib/colors.py

Lines 1175 to 1177 in b01462c

colors : list, array
Sequence of Matplotlib color specifications (color names or RGB(A)
values).

However, when specifying N, str and float are also supported (if one use the source code)

matplotlib/lib/matplotlib/colors.py

Lines 1199 to 1214 in b01462c

if isinstance(colors, str):
self.colors = [colors] * N
self.monochrome = True
elif np.iterable(colors):
if len(colors) == 1:
self.monochrome = True
self.colors = list(
itertools.islice(itertools.cycle(colors), N))
else:
try:
gray = float(colors)
except TypeError:
pass
else:
self.colors = [gray] * N
self.monochrome = True

This means that, e.g., ListedColormap("#aabbcc", N=1) works, but ListedColormap("#aabbcc") does not (there will be weird errors later, like N=7 for the latter). Given that there is monochrome attribute and the documentation of N, one may expect the latter to work as well (if the earlier works). Also, ListedColormap(["#aabbcc"]) will not set the monochrome attribute correctly.

Proposed fix

I think there are three(?) possible solutions:

  • Support scalars/strings when N is not provided (and ideally documents that)
  • Document that if N is not None, the first argument can be a scalar or string (it is maybe easier to do ListedColormap(0.3, N=7) than ListedColormap([0.3]*7) or at least require slightly less Python knowledge)
  • Deprecate scalar/string argument

Bonus: document the color and monochrome attributes
@tacaswell
Copy link
Member

There is also an odd behavior that if you pass a non-iterable as colors with a non-None N instantiation succeeds, but it is super broken:

n [3]: ListedColormap(object(), N=2)
Out[3]: <matplotlib.colors.ListedColormap at 0x7ca5b00d4ce0>

In [4]: ListedColormap(object(), N=2).colors
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
Cell In[4], line 1
----> 1 ListedColormap(object(), N=2).colors

AttributeError: 'ListedColormap' object has no attribute 'colors'

In [5]: ListedColormap(object(), N=2)(.5)
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
Cell In[5], line 1
----> 1 ListedColormap(object(), N=2)(.5)

File /usr/lib/python3.12/site-packages/matplotlib/colors.py:725, in Colormap.__call__(self, X, alpha, bytes)
    702 r"""
    703 Parameters
    704 ----------
   (...)
    722 RGBA values with a shape of ``X.shape + (4, )``.
    723 """
    724 if not self._isinit:
--> 725     self._init()
    727 xa = np.array(X, copy=True)
    728 if not xa.dtype.isnative:
    729     # Native byteorder is faster.

File /usr/lib/python3.12/site-packages/matplotlib/colors.py:1175, in ListedColormap._init(self)
   1173 def _init(self):
   1174     self._lut = np.zeros((self.N + 3, 4), float)
-> 1175     self._lut[:-3] = to_rgba_array(self.colors)
   1176     self._isinit = True
   1177     self._set_extremes()

AttributeError: 'ListedColormap' object has no attribute 'colors'

@tacaswell tacaswell added this to the v3.10.0 milestone Aug 29, 2024
@timhoffm
Copy link
Member

Let's deprecate scalar/string argument. I don't expect is is widely used, and ListedColormap([0.3]*7) is simple enough (we should not make additional APIs just because somebody may not know Python).

@oscargus
Copy link
Member Author

Does that mean that we should deprecate the N argument as well? As colorlist[:N] is as simple? Or is the islice/cycler combo doing something more clever than that?

@timhoffm
Copy link
Member

timhoffm commented Aug 31, 2024 •
edited
Loading

I believe N can go as well. Truncating colors by the user is simple. Repeating a short list multiple times should be the very exception (and should possibly have required explicit user consent as duplicate colors make the mapping ambiguous).

@rcomer rcomer mentioned this issue Sep 7, 2024
Merged
4 tasks
@timhoffm timhoffm modified the milestones: v3.10.0, v3.11.0 Oct 17, 2024
timhoffm added a commit to timhoffm/matplotlib that referenced this issue Nov 12, 2024
@timhoffm
MNT: Refactor from_levels_and_colors()
04707f5 
Primary motivation was to remove the N parameter to ListedColormap (in
preparation of its deprecation
matplotlib#28763 (comment)
-2322791660).
That parameter is not needed here because
`len(colors[color_slice]) == n_data_colors)`, otherwise the ValueError
above would have raised.

Hopefully, this is overall more readable.
@timhoffm timhoffm mentioned this issue Nov 12, 2024
Merged
timhoffm added a commit to timhoffm/matplotlib that referenced this issue Nov 12, 2024
@timhoffm
MNT: Refactor from_levels_and_colors()
44b9873 
Primary motivation was to remove the N parameter to ListedColormap (in
preparation of its deprecation
matplotlib#28763 (comment)
-2322791660).
That parameter is not needed here because
`len(colors[color_slice]) == n_data_colors)`, otherwise the ValueError
above would have raised.

Hopefully, this is overall more readable.
@timhoffm timhoffm mentioned this issue Nov 14, 2024
Open
8 tasks
@timhoffm
Copy link
Member

timhoffm commented Jan 13, 2025 •
edited
Loading

The parameter N is deprecated in #29135. When the deprecation expires, N will be gone, which resolves this issue.

I'm closing as there are no further actions needed. The deprecation will guide people away from the API in question. And when expiring the topic is done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
API: consistency Documentation: API files in lib/ and doc/api Maintenance topic: color/color & colormaps
Projects
None yet
Milestone
v3.11.0
Development

No branches or pull requests
3 participants
@tacaswell @timhoffm @oscargus