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

Check cmap type before comparing to categories #1462

Merged
merged 4 commits into from
Dec 11, 2024

Conversation

Azaya89
Copy link
Contributor

@Azaya89 Azaya89 commented Dec 3, 2024

fixes #1461 where the use of matplotlib colormap objects as cmap argument results in "TypeError: argument of type 'LinearSegmentedColormap' is not iterable...."

import hvplot.xarray  # noqa
from matplotlib import colormaps
import xarray as xr

ds = xr.tutorial.open_dataset("air_temperature")
da = ds.air.isel(time=0)

da.hvplot.image(cmap=colormaps['viridis'])

image

@Azaya89 Azaya89 requested a review from hoxbro December 3, 2024 14:15
@Azaya89 Azaya89 self-assigned this Dec 3, 2024
Copy link

codecov bot commented Dec 3, 2024

Codecov Report

Attention: Patch coverage is 92.85714% with 1 line in your changes missing coverage. Please review.

Project coverage is 88.96%. Comparing base (9078378) to head (1393fd1).
Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
hvplot/util.py 80.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1462      +/-   ##
==========================================
+ Coverage   88.94%   88.96%   +0.01%     
==========================================
  Files          52       52              
  Lines        7781     7812      +31     
==========================================
+ Hits         6921     6950      +29     
- Misses        860      862       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hoxbro hoxbro added this to the 0.11.2 milestone Dec 3, 2024
hvplot/converter.py Outdated Show resolved Hide resolved
@Azaya89 Azaya89 requested a review from ahuang11 December 5, 2024 12:33
hvplot/converter.py Outdated Show resolved Hide resolved
@Azaya89 Azaya89 requested a review from ahuang11 December 9, 2024 15:49
Copy link
Collaborator

@ahuang11 ahuang11 left a comment

Choose a reason for hiding this comment

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

Thanks! I think it fixes the issue.

One thing you could test is whether a categorical cmap as a string and LinearSegmentedColormap actually gets processed as expected.

@Azaya89
Copy link
Contributor Author

Azaya89 commented Dec 11, 2024

One thing you could test is whether a categorical cmap as a string and LinearSegmentedColormap actually gets processed as expected.

Maybe I could do that in another PR since this one has solved the original issue.

Copy link
Member

@maximlt maximlt left a comment

Choose a reason for hiding this comment

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

LGTM!

One thing you could test is whether a categorical cmap as a string and LinearSegmentedColormap actually gets processed as expected.

@Azaya89 has been working on cataloguing the different ways usage and values accepted by cmap/color_key/color/c/etc. Once we have a good reference document, we will be able to more easily identify gaps in the test suite and fix them. Let's do this in another PR to get the fix of this one merged and released.

@maximlt maximlt merged commit c2f9d2b into holoviz:main Dec 11, 2024
9 checks passed
@Azaya89 Azaya89 deleted the azaya-1461 branch December 11, 2024 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants