Skip to content

Deprecate implicit creation of colormaps in register_cmap()#15875

Merged
timhoffm merged 1 commit into
matplotlib:masterfrom
timhoffm:deprecate-register_cmap-data
Dec 9, 2019
Merged

Deprecate implicit creation of colormaps in register_cmap()#15875
timhoffm merged 1 commit into
matplotlib:masterfrom
timhoffm:deprecate-register_cmap-data

Conversation

@timhoffm

@timhoffm timhoffm commented Dec 8, 2019

Copy link
Copy Markdown
Member

PR Summary

register_cmap() could be used either with a Colormap, or by passing data to implicitly create a colormap from.

This PR deprecates the second way in favor of explicitly creating colormaps viaregister_cmap(cmap=LinearSementedColormap(name, data lut)).

Having both ways is redundant, makes the function more complicated, and unnecessarily pulls colormap implementation details (like the lut) into the API of register_cmap(). Given that colormap registration is only done very rarely, the benefit of a sligtly shorter call register_cmap(name, data=data, lut=lut) is negligable. It's even error prone because data has to be passed as kwarg since the second arg is the for this case unused cmap parameter.

PR Checklist

  • Code is Flake 8 compliant
  • Documentation is sphinx and numpydoc compliant
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@timhoffm timhoffm added this to the v3.3.0 milestone Dec 8, 2019
@timhoffm timhoffm added the API: changes Changes to the public API, typically requiring deprecation. label Dec 8, 2019
Comment thread doc/api/next_api_changes/deprecations.rst Outdated
Comment thread doc/api/next_api_changes/deprecations.rst Outdated
Comment thread lib/matplotlib/cm.py Outdated
Comment thread lib/matplotlib/cm.py Outdated
@timhoffm timhoffm force-pushed the deprecate-register_cmap-data branch from 7524ced to 38d7e06 Compare December 8, 2019 12:13

@anntzer anntzer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

postci

@tacaswell tacaswell left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Needs a re-base to resolve conflict in deprecation docs.

@timhoffm timhoffm force-pushed the deprecate-register_cmap-data branch from 38d7e06 to eb61e47 Compare December 8, 2019 21:09
@timhoffm

timhoffm commented Dec 9, 2019

Copy link
Copy Markdown
Member Author

Rebase done. Taking the liberty to self-merge based on the two positive reviews.

@timhoffm timhoffm merged commit 0e0d72a into matplotlib:master Dec 9, 2019
@timhoffm timhoffm deleted the deprecate-register_cmap-data branch December 9, 2019 20:35
tacaswell added a commit to tacaswell/matplotlib that referenced this pull request Sep 24, 2020
tacaswell added a commit to tacaswell/matplotlib that referenced this pull request Sep 25, 2020
This was deprecated in 3.3 via matplotlib#15875

Enforce type of cmap as Colormap because we may not know that the
object registered is of the wrong type until well after it was
registered, defensively check here.

We could do a duck-type check, but this is much simpler and we can
cross the bridge of writing a duck-type checking function when someone
has a use case for registering
compatible-but-not-derived-from-Colormap instances.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

API: changes Changes to the public API, typically requiring deprecation. topic: color/color & colormaps

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants