Lazy loading colormaps #343
Conversation
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
|
hmm this test that is failing is passing locally.. strange |
cvanelteren
force-pushed
the
fix-cmap-loading
branch
from
1c6a465 to
6e3278d
Compare
cvanelteren
force-pushed
the
fix-cmap-loading
branch
from
b187ee4 to
f2dae18
Compare
| # by overriding matplotlib's behavior | ||
| for name in tuple(self._cmaps.keys()): | ||
| self.register(self._cmaps[name], name=name) | ||
| super().__init__({k.lower(): v for k, v in kwargs.items()}) |
There was a problem hiding this comment.
Why switch to k.lower() here?
There was a problem hiding this comment.
We perform a lower command internally later when the colormap is registered.
| # such that ultraplot knows what these objects are. We piggy back on the registering mechanism | ||
| # by overriding matplotlib's behavior | ||
| for name in tuple(self._cmaps.keys()): | ||
| self.register(self._cmaps[name], name=name) |
There was a problem hiding this comment.
I guess we are no longer using the register method?
There was a problem hiding this comment.
Ah good catch. We do need it as we override the ColormapRegistery. Moved the lazy loading from file to a private function to make this distinction clearer and still rely on register
ultraplot/colors.py
Outdated
| raise ValueError( | ||
| f"Invalid colormap type {type!r} for key {key!r} in file {path!r}. " | ||
| "Expected 'continuous' or 'discrete'." | ||
| ) | ||
|
|
||
| if cmap: | ||
| if is_default and cmap.name.lower() in CMAPS_CYCLIC: | ||
| cmap.set_cyclic(True) | ||
| self._cmaps[key] = cmap | ||
| value = cmap | ||
| else: # failed to load | ||
| # remove from registry to avoid trying again | ||
| del self._cmaps[key] | ||
| raise KeyError(f"Failed to load colormap {key!r} from {path!r}") |
There was a problem hiding this comment.
The previous code warned on failures at least according to the keyword. This code appears to raise an error?
There was a problem hiding this comment.
I will change it back to a warn.
2 participants
This PR changes the colormap loading to lazy loading -- effectively reducing the import time from
0.38sto0.00082sfor the colormaps.