-
Notifications
You must be signed in to change notification settings - Fork 12
feat: add cmap_kwargs to Colormap, to pass kwargs to parametrized colormaps #134
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
Conversation
…("cubehelix", cmap_kwargs: dict[str, Any]) API
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #134 +/- ##
==========================================
+ Coverage 95.55% 95.56% +0.01%
==========================================
Files 168 168
Lines 2181 2186 +5
==========================================
+ Hits 2084 2089 +5
Misses 97 97 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
thanks @alisterburt! love the idea, would prefer to avoid a new registry though... and i think we should be able to avoid it. in the Lines 232 to 234 in 1697c07
and then if cmap_kwargs and not callable(info.data):
raise ShameOnYou
if isinstance(info.data, list):
...
else:
data = partial(info.data, **cmap_kwargs) if callable(info.data) else info.data
stops = _parse_colorstops(data)thoughts? |
|
(and that shows that there are many more than just cubehelix) In [5]: {x.name for x in Colormap.catalog().values() if callable(x.data)}
Out[5]: {'ocean', 'afmhot', 'gnuplot', 'heat', 'flag', 'prism', 'cubehelix', 'rainbow', 'fire', 'gnuplot2', 'grv'}though, not all of those accept params... which raises the question: when should "bad" params be detected? I would think at init-time... but we might need to force the creation of a small lut to check it |
|
thanks for the fast turnaround on review - all makes sense, out and about today but will try to find some time in the coming days to get this over the line |
|
I'm also more than happy to do it if you were just opening the PR to be polite and don't care either way :) |
|
@tlambert03 sorry for the lack of response here - totally on board with all of your suggestions and unlikely to get to it myself on any reasonable timescale so please clobber at will! |
and expose cubehelix with
Colormap("cubehelix", cmap_kwargs: dict[str, Any])APIcloses #19