Skip to content

Conversation

@utsmok
Copy link

@utsmok utsmok commented May 21, 2024

Fixes breaking change -- get_cmap cannot be imported from matplotlib.cm, as it was removed from matplotlib.cm as a standalone function as can be seen in the history of matplotlibs' cm module here.

as suggested I changed the import to use matplot.pyplot instead. If this is not changed polyfuzz will raise an ImportError during importing.

Fixes breaking change -- get_cmap cannot be imported from matplotlib.cm, as it was removed from matplotlib.cm as a standalone function as can be seen in the history of matplotlibs' cm module here: matplotlib/matplotlib@7f23ee1

as suggested I changed the import to use matplot.pyplot instead. 
If this is not changed polyfuzz will raise an ImportError during importing.
@MaartenGr
Copy link
Owner

Thanks for sharing! Does this change also work with the minimum matplotlib version used in this package? This change will only work if it follows the minimum reqs:

"matplotlib>= 3.2.2",

@utsmok
Copy link
Author

utsmok commented May 22, 2024

Good point! The deprecation of this cmap function was put into matplotlib 2 years ago, and around the same time a note was added that this alternative will remain in the codebase, see this commit. I dug through the releases, and the pyplot.get_cmap function was initially added in version 3.6.0 as far as I can tell; so upping the minimum matplot version to 3.6.0 would be best I'd say. It's also possible to use other functions to get access to the colormaps as indicated by the deprecation messages -- but this will require some more changes to the code instead of just importing the function from another module.

@lukedavisseo
Copy link

I literally ran into this issue 30 minutes ago so thanks for flagging, @utsmok!

@MaartenGr
Copy link
Owner

I dug through the releases, and the pyplot.get_cmap function was initially added in version 3.6.0 as far as I can tell; so upping the minimum matplot version to 3.6.0 would be best I'd say.

Awesome, thanks for taking the time to go through the releases. That's highly appreciated!
I agree, simply upping the minimum version would be okay considering it is already fairly low.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants