Matplotlib v3.9 compatibility - #3394
Conversation
Introduce a new utility module that centralizes colormap access, listing, registration, and unregistration across Matplotlib API versions. The new helpers prefer modern `matplotlib.colormaps` APIs when available and fall back to legacy `matplotlib.cm`/`pyplot` behavior, with deprecation markers on legacy paths to support the migration.
Adds a `stacklevel` argument to `deeplabcut.core.deprecation.deprecated()` and uses it when emitting `DLCDeprecationWarning`. This keeps the default behavior (`stacklevel=2`) while allowing wrapper/legacy shim layers to bump the level so warnings point to the real caller.
Replaced direct `plt.cm.get_cmap(...)` calls across visualization and plotting modules with `deeplabcut.utils.matplotlib_future_mode.get_colormap`. This centralizes colormap access, keeps behavior consistent across TensorFlow/PyTorch/GUI/3D paths, and aligns with Matplotlib future-mode compatibility.
…_names Previously it duplicated the legacy lookup inline (list(plt.colormaps())) instead of calling the @deprecated-decorated _legacy_get_colormap_names(), so the deprecation warning never fired on old Matplotlib installs, unlike register_colormap/unregister_colormap.
_legacy_register_colormap/_legacy_unregister_colormap/_legacy_get_colormap_names now report their public wrapper names (e.g. register_colormap) in deprecation warnings instead of the private helper name.
update all import sites accordingly
|
@C-Achard, great efforts, good plan. When working on this PR myself as well, I realized that adding a whole compat layer might not be necessary if the single solution it might be helpful to merge #3430 first! |
|
(pushed my additional changes now but feel free to revert) |
There was definitely some future-proofing/extra caution in doing it this way, as I was concerned we may uncover other matplotlib-related issues that would arise/need to be added later. Hopefully the current layer is not too bloated/redundant, we can definitely trim if you think there is no need for it. I do agree it should be "disposable" ideally and removed later, so that would add some weight towards making it leaner, but at the same time it's already mostly there. Perhaps, on the other hand, would you see any extensions to it that could be needed, or is it also clear for you that only the cmap getter is needed ? If yes then I would definitely not shrink it. If no we can remove some. Let me know what you thinkl! |
|
I agree with all latest changes, thanks a lot for the testing additions |
|
Let's merge #3430 first an then revisit this PR |
Add an internal helper to fetch colormap names without triggering the DLC deprecation wrapper, and reuse it in the legacy name getter. Update legacy colormap registration to pre-check for existing names and raise a clear ValueError when `force=False`, restoring expected duplicate handling
Add `remove_artists()` to `matplotlib_compat` and use it in the tracklet toolbox and visualization cleanup paths. This avoids deprecated `ArtistList` concatenation in Matplotlib 3.7+ and preserves safe removal by snapshotting each artist sublist before calling `remove()`.
Avoid private import of the logger
Add regression tests for the legacy colormap registration fallback, including duplicate-name handling when `force` is disabled. This also adds direct coverage for `remove_artists` and `silence_axes_logger` using real Matplotlib axes and logger objects, helping catch upstream API or behavior changes in the compatibility layer.
|
@deruyter92 Hopefully this version addresses everything related to the version pin, let me know what you think |
deruyter92
left a comment
There was a problem hiding this comment.
@C-Achard, thanks for the work on this and also looking into the extra cases (e.g. artists)
Sorry to push back a bit again: but do we really need a compatibility layer? As mentioned before I think it would be better to just do the migration now instead of adding a migration layer that we currently do not need.
i.e. if plt.cm.get_cmap -> plt.get_cmap fixes the current breakage + we add a useful silence_axes_logger I think we are more than set for releasing the upper-bound right?
regarding remove_artists, to me it seems that there is really no compatibility requirement. But maybe I am overlooking things. Even then, I would prefer just adjusting the usage to the recommended pattern instead of adding helpers that redo that logic.
Let me know if you honestly think that it is really needed. Otherwise it's just extra code to maintain (and it pretends there is a compatibility problem when the real fix is already the upstream-supported drop-in).
Replaces `deeplabcut.utils.matplotlib_compat` with `matplotlib_utils` and narrows it to the shared `silence_axes_logger` helper plus `AXES_LOGGER_NAME`. Removes the old colormap and artist-removal compatibility helpers and rewrites tests to target the new module and its logger behavior.
This refactor removes broad usage of `deeplabcut.utils.matplotlib_compat` across visualization and tracking modules by switching to native Matplotlib APIs (`plt.get_cmap`) and local artist removal logic. It also updates 3D calibration/plotting to import `silence_axes_logger` from `matplotlib_utils`, reducing compatibility-wrapper dependencies while preserving existing plotting behavior.
This change trims the helper and tests down to the essential behavior: the axes logger is still retrieved by name and its level is set, while the extra explanatory comments and redundant explicit-level test are removed. The result is a smaller, clearer implementation without changing the intended logger suppression behavior.
This change avoids unsupported list concatenation on Matplotlib artist collections when clearing axes. The code now materializes combined lists explicitly before iterating, preventing errors in tracklet and visualization cleanup paths.
Scope
Migrates off
plt.cm.get_cmap, which matplotlib removed in 3.9.Changes
plt.cm.get_cmap→plt.get_cmapat the 8 call sites. Only change the 3.9 removal actually required (pin matplotlib version #2583, Matlplotlib after installed by CONDA missing matplotlib.cm.cmap #2581).matplotlib_utils.silence_axes_logger()replacesfrom matplotlib.axes._axes import _login the two 3D modules. Resolves the same logger by name instead of importing a private symbolerase_artists/clean_collectionssnapshot theAxeschildren sublists explicitlyNotes
Earlier revisions of this PR added a
matplotlib_compatlayer.plt.get_cmapis undeprecated in 3.9, 3.10 and onmain, andArtistListconcatenation was never deprecated, so there was no compatibility problem for a layer to solve.deprecated(name=, stacklevel=)now has no in-repo caller. Kept as useful and tested, but happy to drop it.