FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Ensure name in set_cmap is contained in cmap_d by ianhi · Pull Request #17012 · matplotlib/matplotlib · GitHub

  1. matplotlib
  2. matplotlib

Repository navigation

Ensure name in set_cmap is contained in cmap_d #17012

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

Closed
ianhi wants to merge 2 commits into from
Conversation 11 Commits 2 Checks 0 Files changed
Closed

Ensure name in set_cmap is contained in cmap_d #17012

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d0abb3c
modify set_cmap to always use the correct cmap name
ianhi Apr 3, 2020
cb02f11
flake8 changes
ianhi Apr 3, 2020
File filter

Filter by extension

Filter by extension .py  (1) All 1 file type selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
14 changes: 10 additions & 4 deletions lib/matplotlib/pyplot.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
import numpy as np

# We may not need the following imports here:
from matplotlib.colors import Normalize
from matplotlib.colors import Normalize, Colormap
from matplotlib.lines import Line2D
from matplotlib.text import Text, Annotation
from matplotlib.patches import Polygon, Rectangle, Circle, Arrow
Expand Down Expand Up @@ -2069,11 +2069,17 @@ def set_cmap(cmap):
matplotlib.cm.register_cmap
matplotlib.cm.get_cmap
"""
cmap = cm.get_cmap(cmap)
cbook._check_isinstance((str, Colormap), cmap=cmap)
if isinstance(cmap, str):
name = cmap
cmap = get_cmap(name)
rc('image', cmap=name)
elif isinstance(cmap, Colormap):
if cmap.name not in cm.cmap_d.keys():

greglucas Apr 3, 2020

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

You don't need the .keys() when checking if something is in a dictionary.
register_cmap(cmap.name, cmap)
rc('image', cmap=cmap.name)

greglucas Apr 3, 2020

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Here you could get into another tricky situation.

Say someone has already registered a different Colormap under the same name. So, the cmap.name is in the dictionary already, but it is actually pointing to a different colormap and this wouldn't set the global cmap to what you're expecting either.

ianhi Apr 3, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Ah, this is tricky. Perhaps:

if cmap.name not in cm.cmap_d:
    register_cmap(cmap.name, cmap)
elif cmap is not get_cmap(cmap.name):
    raise some sort of error/warning

and in that error/warning explain that they should use register_cmap or change the name attribute of their colormap?

Also good to note that as originally written this function falls into the same trap. Though I suppose it's not super desirable to maintain backwards compatibility with a silent bug.

greglucas Apr 3, 2020

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Agreed, something like this would be desirable going forward to make it a little more clear what is going on and raising rather than silently doing something unexpected!

rc('image', cmap=cmap.name)
im = gci()

if im is not None:
im.set_cmap(cmap)

Expand Down
Back | FazBrowse Home | New Git URL