Fix ListedColormap N deprecation and add dependency version constraints - #121
Open
xylar wants to merge 2 commits into
Open
Fix ListedColormap N deprecation and add dependency version constraints#121xylar wants to merge 2 commits into
ListedColormap N deprecation and add dependency version constraints#121xylar wants to merge 2 commits into
Conversation
Passing `N` to `ListedColormap` is deprecated since matplotlib 3.11 and will be removed in 3.13. In each case here `N` was already equal to the number of colors being passed in, which is what matplotlib uses by default, so the argument can simply be dropped without any change in the resulting colormaps. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The `matplotlib<3.5` branch of `_register_cmap()` has never actually worked: `cmocean/cm.py` does `import matplotlib as mpl`, which does not make the `matplotlib.cm` submodule available, so the branch raised `AttributeError: module 'matplotlib' has no attribute 'cm'`. A bare `import cmocean` therefore failed outright on matplotlib<3.5. The test suite missed this because `tests/test_cmocean.py` imports `matplotlib.pyplot` first, which pulls in `matplotlib.cm` as a side effect and makes the attribute resolve. Import the submodule explicitly so the fallback works on its own. With that fixed, cmocean imports cleanly and registers all 110 `cmo.*` colormaps with identical values on every matplotlib from 3.1.2 (the oldest with a CPython 3.8 wheel, matching `python_requires=">=3.8"`) through 3.11.1, and on every numpy from 1.17.3 through 2.5.1. Neither dependency imposes a lower bound of its own, so none is added; where old numpy does fail it is matplotlib's own `numpy>=1.20` check that rejects it, which is matplotlib's constraint to express, not cmocean's. The same goes for `packaging`, whose `Version` class behaves identically back to at least 16.8. The one constraint that is worth stating is on setuptools, and `setup_requires` is the wrong place for it: it is deprecated and warns about `fetch_build_eggs`. Declare it in a PEP 517 `[build-system]` table instead and drop `setup_requires`. `setuptools>=64` is what gets editable installs the real PEP 660 path rather than the deprecated `setup.py develop` fallback. Removing the unused `PyTest` command class (it was never wired up through `cmdclass`) drops the last reference to `setuptools.command.test`, which setuptools has disabled and warns about. The likewise ignored `tests_require` is replaced by a `test` extra, so `pip install cmocean[test]` pulls in pytest; a `[project.optional-dependencies]` table in `pyproject.toml` would have been the more modern home, but a partial `[project]` table is rejected, so that would mean moving all metadata out of `setup.py`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Author
|
I see that this is somewhat redundant with #114. I hadn't seen that PR before making mine. Sorry about that. But I think this one might be the better replacement because it fixes two issues. |
Author
|
@kthyng, is this something you have time to review? It would be much appreciated, if so. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix the
ListedColormapdeprecation warningIn testing various downstream packages, I've been seeing that importing
cmoceancurrently emitsMatplotlibDeprecationWarnings under matplotlib 3.11:In the unit tests here, there are 88 such warnings (4 per colormap × 22 colormaps). In every one of these calls
Nwas already equal to the number of colors being passed in, which is exactly what matplotlib uses whenNis omitted, so the argument is simply dropped. The registered colormaps are unchanged: all 110cmo.*entries keep the sameNand identical RGBA values as before.Fix the
matplotlib<3.5fallbackI was checking to see if a lower bound was needed on
matplotlibornumpy. It turns out that none is needed.Testing the version constraints turned up a latent bug. The
matplotlib<3.5branch of_register_cmap()does not seem to work as intended:cmocean/cm.pydoesimport matplotlib as mpl, which does not make thematplotlib.cmsubmodule available, so the branch raisedAttributeError: module 'matplotlib' has no attribute 'cm'and a bareimport cmoceanfailed outright on matplotlib<3.5. The test suite missed this becausetests/test_cmocean.pyimportsmatplotlib.pyplotbefore importing cmocean, which pulls inmatplotlib.cmas a side effect and makes the attribute resolve. The submodule is now imported explicitly so the fallback should now also work as expected (however unlike it may be that anyone actually needs it).Version constraints
Thorough testing showed no lower bound is needed for numpy, matplotlib or packaging.
I added
setuptools >=64to a newpyproject.toml[build-system]table rather than insetup_requires, which is itself deprecated and warns aboutfetch_build_eggs. Version 64 is where editable installs are not using a deprecated fallback, so it is a sensible lower bound.Removing the unused
PyTestcommand class (not wired up throughcmdclass) drops the last reference tosetuptools.command.test, which setuptools has disabled and warns about, and which would otherwise have forced asetuptools<72cap. The likewise ignoredtests_requireis replaced by atestextra alongside the existingplotsone, sopip install cmocean[test]pulls in pytest.Testing
pytestpasses with no warnings at all under matplotlib 3.11.1, numpy 2.5.1, Python 3.14.Many other versions were also checked as part of looking for a lower bound, again with no warnings.
AI help
These code changes were made with help from Claude Opus 5, but I have looked over the changes myself and stand by them.