search

holoviz / colorcet

A set of useful perceptually uniform colormaps for plotting scientific data
http://colorcet.holoviz.org
Other
682 stars 52 forks source link

Only register if cmap not in colormaps #108

Closed hoxbro closed 1 year ago

hoxbro commented 1 year ago

This comes up sometimes when debugging with panel serve --autoreload and changing a file in the holoviews/hvplot repo.

This has only come up for me when debugging, not actual use.

Steps to reproduce: 1) Make a file called with import colorcet 2) panel serve --autoreload that file 3) Make some changes in colorcet/__init__.py and reload the page

ValueError: A colormap named "cet_diverging_isoluminant_cjm_75_c23" is already registered.

Traceback (most recent call last):
  File "/home/shh/miniconda3/envs/holoviz/lib/python3.10/site-packages/bokeh/application/handlers/code_runner.py", line 231, in run
    exec(self._code, module.__dict__)
  File "/home/shh/Development/holoviz/development/dev_hvplot/tmp.py", line 1, in <module>
    import colorcet  # noqa
  File "/home/shh/Repos/holoviz/colorcet/colorcet/__init__.py", line 551, in <module>
    m_diverging_isoluminant_cjm_75_c23 = mpl_cm('diverging_isoluminant_cjm_75_c23',diverging_isoluminant_cjm_75_c23)
  File "/home/shh/Repos/holoviz/colorcet/colorcet/__init__.py", line 92, in mpl_cm
    register_cmap("cet_"+name, cmap=cm[name])
  File "/home/shh/Repos/holoviz/colorcet/colorcet/__init__.py", line 70, in register_cmap
    colormaps.register(cmap, name=name)
  File "/home/shh/miniconda3/envs/holoviz/lib/python3.10/site-packages/matplotlib/cm.py", line 136, in register
    raise ValueError(
ValueError: A colormap named "cet_diverging_isoluminant_cjm_75_c23" is already registered.
ianthomas23 commented 1 year ago

It might be worth adding a new test to confirm that a ValueError is raised as in the example.

hoxbro commented 1 year ago

Added unit test. The failing tests on Python 3.6 are unrelated.

jbednar commented 1 year ago

See https://github.com/holoviz/colorcet/issues/96 for issues with this approach. Can those be addressed?

hoxbro commented 1 year ago

I'm not sure I follow. Why is there an issue with this approach?

jbednar commented 1 year ago

See https://github.com/holoviz/colorcet/issues/96#issuecomment-1276576516 . Skipping registering the map is dangerous unless you verify that the map is indeed the same, not just the same name.

jbednar commented 1 year ago

I'd be fine with skipping it if the name is the same and a Python equality test indicates that it's the same object, which should be true if it's just about reloading the same code. ETA: Looking again at the code, I now see a line that does check that it's the same object. In that case, this looks fine! Sorry for the noise!

jbednar commented 1 year ago

Python 3.6 tests were failing, which we'll have to consider when we do the next release. I assume that's to do with instantiating an environment rather than an actual problem, and I'm happy for this package to be installable on 3.6 even if there are aspects of the docs or other build steps that won't pass on 3.6.

maximlt commented 1 year ago

That package won't be installable on Python 3.6 after the next release https://github.com/holoviz/colorcet/pull/110

jbednar commented 1 year ago

Ah, yes. I guess it's fine; "colorcet" as a package should be available for Python 3.6, but the very latest versions need not be.