MaartenGr / PolyFuzz

Fuzzy string matching, grouping, and evaluation.
https://maartengr.github.io/PolyFuzz/
MIT License
725 stars 68 forks source link

Fix deprecated matplotlib import in metrics.py #78

Open utsmok opened 2 months ago

utsmok commented 2 months ago

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.

MaartenGr commented 2 months ago

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:

https://github.com/MaartenGr/PolyFuzz/blob/5d0734b093ece13e3e0e95dd3a26549f014497a8/setup.py#L21

utsmok commented 2 months ago

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 commented 2 months ago

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

MaartenGr commented 2 months ago

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.