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.

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

Fix for importing mpl colormaps with recent versions of matplotlib #1077

Merged
merged 5 commits into from
Dec 4, 2023
Merged

Fix for importing mpl colormaps with recent versions of matplotlib #1077

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
83228e1
Fix for importing mpl colormaps with recent versions of matplotlib
ejeschke Dec 2, 2023
d1b492e
remove blank line
ejeschke Dec 2, 2023
088fa76
Remove support for matplotlib < 3.4
ejeschke Dec 2, 2023
eda38e1
bump matplotlib recommended version
ejeschke Dec 2, 2023
ed8d25d
Remove code for ignoring MatplotlibDeprecationWarning
ejeschke Dec 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/WhatsNew.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ Ver 5.0.0 (unreleased)
- Fix for a logger annoyance message when mousing over far edge of image
- Updates for deprecations in numpy 2.0
- Fix for missing menubar on some versions of Qt and Mac OS X
- Fix for importing mpl colormaps with recent versions of matplotlib

Ver 4.1.0 (2022-06-30)
======================
Expand Down
7 changes: 3 additions & 4 deletions ginga/cmap.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13309,8 +13309,7 @@ def add_matplotlib_cmap(cm, name=None):
def add_matplotlib_cmaps(fail_on_import_error=True):
"""Add all matplotlib colormaps."""
try:
import matplotlib.pyplot as plt
from matplotlib import cm as _cm
from matplotlib import colormaps as _mpl_cm
if MPL_LT_3_4:
from matplotlib.cbook import mplDeprecation as MatplotlibDeprecationWarning
else:
Copy link
Owner Author

Choose a reason for hiding this comment

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

@pllim, should we drop support for mpl < 3.4 and git rid of this workaround needed for suppressing MatplotlibDeprecationWarning ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Matplotlib is at 3.8 now. Matplotlib 3.3 has not been touched since early 2021. Do you still have any users stuck with matplotlib < 3.4? If not, probably less work to drop it.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Sounds good. I'll remove the support from this module for that.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Should also bump this?

ginga/setup.cfg

Line 60 in f0eecdf

matplotlib>=2.1

Copy link
Owner Author

Choose a reason for hiding this comment

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

good catch

Expand All @@ -13322,14 +13321,14 @@ def add_matplotlib_cmaps(fail_on_import_error=True):
return

# NOTE: Update if matplotlib has new public API for this.
for name in plt.colormaps():
for name in _mpl_cm:
if not isinstance(name, str):
continue
try:
# Do not load deprecated colormaps
with warnings.catch_warnings():
warnings.simplefilter('error', MatplotlibDeprecationWarning)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the deprecation warning catching should be removed. It should no longer happen.

Copy link
Owner Author

Choose a reason for hiding this comment

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

@pllim, because those particular color maps have been removed some time ago? I guess we can add the code back if they deprecate more color maps in the future. Wondering why they deprecated color maps...

Copy link
Collaborator

Choose a reason for hiding this comment

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

@ejeschke , I don't follow matplotlib development closely, so not sure. But I do not see any warning now using the matplotlib I have locally. Do you?

I guess does not hurt to keep it as well.

cm = _cm.get_cmap(name)
cm = _mpl_cm[name]
add_matplotlib_cmap(cm, name=name)
except Exception as e:
if fail_on_import_error:
Expand Down