Resolve #60, #56 - Allow multiple aliases, update colormaps from colorcet.m (second attempt) #63

randallpittman · 2021-02-22T18:42:04Z

This is a second attempt at #62.

Resolves Allow more aliases #60 by changing aliases in CET_to_py.py to a dict of lists. This renders aliases_v2 unnecessary. Minor changes necessary to make CET_to_py.py and then colorcet/__init__.py work with a dict of lists.
Resolves Update colorcet to upstream, especially new Gouldian Parula replacement #56 by the scripts make_csvs_from_colorcet.m and CET_merge.py, along with instructions in README_assets.md. The scripts were used to generate new CSVs for new colormaps and update CET_to_py.py which in turn were used to update colorcet/__init__.py with the new colormaps and aliases.
aliases in CET_to_py.py is now a dict of lists. This renders the aliases_v2 dict unnecessary, as those can be added in to aliases as additional values.
Ultimately I made sure that no existing colormaps or aliases are overridden but do allow for new aliases for existing colormap (e.g. "heat" == "fire", "gray" == "gray", etc.). Tests check out, and swatches look good in the example notebooks. This took a few iterations to figure out programmatically in CET_merge.py but I think I finally got it right.
New maps are in assets/CETperceptual_csv_0_1_v3. It looked like for each existing cyclical colormap there was also one with 0.25 shift so I followed suit and included similarly-shifted maps along with the non-shifted maps (e.g. CET-C7s etc.)

I think examples/assets/images/named.png needs to be updated, but I couldn't get examples/assets/write_named.py to run right on my machine. I got these warnings and the maps didn't plot right at all:

WARNING:bokeh.io.export:There were browser warnings and/or errors that may have affected your export
WARNING:bokeh.io.export:http://localhost:5006/static/extensions/panel/css/json.css - Failed to load resource: net::ERR_CONNECTION_REFUSED
WARNING:bokeh.io.export:http://localhost:5006/static/extensions/panel/css/widgets.css - Failed to load resource: net::ERR_CONNECTION_REFUSED
WARNING:bokeh.io.export:http://localhost:5006/static/extensions/panel/css/alerts.css - Failed to load resource: net::ERR_CONNECTION_REFUSED
WARNING:bokeh.io.export:http://localhost:5006/static/extensions/panel/css/markdown.css - Failed to load resource: net::ERR_CONNECTION_REFUSED
WARNING:bokeh.io.export:http://localhost:5006/static/extensions/panel/css/dataframe.css - Failed to load resource: net::ERR_CONNECTION_REFUSED
WARNING:bokeh.io.export:http://localhost:5006/static/extensions/panel/css/card.css - Failed to load resource: net::ERR_CONNECTION_REFUSED

It should be stated that I'm not a user of holoviews or bokeh, so I might have something set up wrong.

I reworked get_aliases() a bit to just keep trying to get more aliases until no more are found. I hope the result is OK in terms of the order of results returned. I reworked test_get_aliases() to use sets and therefore not care about the order of the maps returned by get_aliases().

Conflict output of CET_merge.py showing old aliases and mappings retained that are different from colorcet.m:

#
# ## NOTICE: Found the following aliases conflicts, with old alias assignment retained over new:
# ## alias, old_descriptorname, new_descriptorname
# bgyw, linear_bgyw_15_100_c
10000
68, linear_bgyw_20_98_c66
# bmw, linear_bmw_5_95_c89, linear_bmw_5_95_c86
# bmy, linear_bmy_10_95_c78, linear_bmy_10_95_c71
# kbc, linear_blue_5_95_c73, linear_kbc_5_95_c73
# kgy, linear_green_5_95_c69, linear_kgy_5_95_c69
# rainbow, rainbow_bgyr_35_85_c73, rainbow_bgyrm_35_85_c69
#
# ## NOTICE: Found the following mapping conflicts, with the CET- name assigned
# ##         to the original map over the new:
# ## CET_name, old_descriptorname, new_descriptorname
# CET-L16, linear_kbgyw_5_98_c62, linear_kbgyw_10_98_c63
#

… from colorcet.m

…replacement

randallpittman · 2021-02-22T22:11:02Z

After a lot of second-guessing I think I'm going to press the proverbial big red button now. Hopefully no more commits now!

jbednar · 2021-02-23T17:32:15Z

Thanks for all your hard work here! I'll review it as soon as I get a chance.

jbednar

Looks great! I've made scattered comments below, but there are two main issues:

Is it possible to preserve the ordering such that existing colormaps do not move locations in the new generated __init__.py file? I want to ensure that no existing colormap values have changed (unless they somehow had an actual error), but it's not feasible to diff that file because the colormaps are now in a different order.
I've made notes about various colormaps that now have multiple short names. I guess the idea here is to match the names provided upstream, but I don't think it's useful to do that unless again there was something wrong with the original short name (e.g. that it matches the name of another different colormap from elsewhere, like blues). Barring such a reason, I'd prefer to keep it down to one long name and one short name per colormap, wherever possible.

.gitignore

assets/CET_merge.py

jbednar · 2021-02-23T16:58:06Z

assets/CET_merge.py

@@ -0,0 +1,100 @@
+# CET_merge.py - create updated `aliases` and `mapping` for CET_to_py.py


Can you explain a bit more at the top of this file, with something like "This script should be run when incorporating upstream changes to colormaps, allowing additional colormaps provided at XXXX to be added safely to colorcet without disrupting existing colormaps."

And then I guess refer to README_assets.md, which documents the whole process.

Sure, I can do that.

jbednar · 2021-02-23T17:00:10Z

assets/CET_to_py.py

+    diverging_gwr_55_95_c38                         = ['gwr'],
+    diverging_gwv_55_95_c39                         = ['gwv'],
+    diverging_linear_bjr_30_55_c53                  = ['bjr'],
+    diverging_linear_bjy_30_90_c45                  = ['bjy', 'divbjy'],


What's the reason for the extra alias divbjy?

So all aliases are automatically pulled from colorcet.m (I didn't add any aliases on my own). colorcet.m links the following names: { 'D7' 'D07' 'BJY' 'DIVBJY' } to the descriptorname and map diverging-linear_bjy_30-90_c45_n256.

jbednar · 2021-02-23T17:01:24Z

assets/CET_to_py.py

+    linear_bgy_10_95_c74                            = ['bgy'],
+    linear_bgyw_15_100_c68                          = ['bgyw'],
+    linear_blue_5_95_c73                            = ['kbc', 'linear_kbc_5_95_c73'],
+    linear_blue_95_50_c20                           = ['depth', 'blues'],  # mpl


Here I guess depth is preferred as a name over blues because there are lots of different blues colormaps around? Makes sense, though I don't know why it would be called depth in particular.

Again, in colorcet.m, L12, and DEPTH are aliases for linear_blue_95-50_c20_n256, which were found with make_csvs_from_colorcet.m and merged in CET_merge.py. I guess it also happens to be the same as blues from matplotlib?

jbednar · 2021-02-23T20:13:19Z

assets/CET_to_py.py

+    linear_gow_65_90_c35                            = ['geographic2'],
+    linear_green_5_95_c69                           = ['kgy', 'linear_kgy_5_95_c69'],
+    linear_grey_0_100_c0                            = ['grey', 'gray'],  # mpl
+    linear_grey_10_95_c0                            = ['reducedgrey', 'dimgray'],


Not sure what 'reducedgrey' adds as an alias here; seems like that will just make the space of names less usable for e.g. GUI colormap selector widgets, with several indistinguishable options.

Same comment as above. reducedgrey is automatically imported from colorcet.m.

jbednar · 2021-02-23T20:13:32Z

assets/CET_to_py.py

+    linear_kbgoy_20_95_c57                          = ['gouldian'],
+    linear_kbgyw_5_98_c62                           = ['kbgyw'],
+    linear_kry_0_97_c73                             = ['kry', 'yellowheat'],
+    linear_kryw_0_100_c71                           = ['kryw', 'heat', 'fire'],


Why 'heat'?

See previous comments.

assets/README_assets.md

jbednar · 2021-02-23T20:54:05Z

If preserving the ordering requires miracles, then I guess what's sufficient is a script we can paste into this PR with its output, where the script imports the new version here and the latest released version of colorcet and reports whether any floating-point values differ for any previously defined colormap.

randallpittman · 2021-02-23T21:22:35Z

@jbednar:

If preserving the ordering requires miracles, then I guess what's sufficient is a script we can paste into this PR with its output, where the script imports the new version here and the latest released version of colorcet and reports whether any floating-point values differ for any previously defined colormap.

These scripts and this PR do not modify or replace any existing colormaps. Only new maps were added, as well as some new aliases for existing maps that were found in colorcet.m.

That said, I think I could probably modify CET_merge.py to preserve the previous order. I'll give it a bit of time and get back to you.

EDIT: The original order is dependent on the order returned by os.listdir, as the maps are put into __init__.py by traversing through the folders of CSVs. That said, I think I can hack a workaround to preserve the existing order.

randallpittman · 2021-02-23T21:27:33Z

@jbednar

I could re-work this so only one short name is kept per map.

Come to think of it, one thing that isn't ideal about the existing implementation the varying filenames for colormap CSV files. They really should all be named with the "descriptor name", e.g. linear_kryw_0-100_c71, as that is the full identity of the colormap.

Again, I'll think about it and get back to you.

jbednar · 2021-02-23T22:12:01Z

The CSV files really should all be named with the "descriptor name", e.g. linear_kryw_0-100_c71

Agreed. That's the full, canonical name, as far as colorcet is concerned, even if upstream has shifted to a different naming style.

These scripts and this PR do not modify or replace any existing colormaps. Only new maps were added, as well as some new aliases for existing maps that were found in colorcet.m.

Right; just need to demonstrate that equality here in this PR before merging, either by preserving the original ordering (in which case git will be able to diff __init__.py) or by separately pasting an example of comparing before and after numerically.

I think examples/assets/images/named.png needs to be updated, but I couldn't get examples/assets/write_named.py it to run right on my machine. I got these warnings and the maps didn't plot right at all:

To run that file it should be sufficient to do conda install -c pyviz holoviews bokeh selenium ; conda install -c conda-forge firefox geckodriver. It did need updates to match a recent change in Panel, so I added linked_axes=False and balanced the columns of output (since their lengths differed now) and pushed the revised script and its current PNG output to your PR. If the colormap names change, it will still need updating, but I can do that after this PR is merged if it's difficult for you to run the script locally.

BTW, studying named.png, rainbow2 seems visually identical to rainbow, though I can see from the parameters it differs slightly. Unless there is a very strong reason to retain it I'd remove the short name to avoid confusion; people will wonder why there are two nearly identical colormaps for them to choose from. rainbow3 and rainbow4 are clearly distinct, and thus deserve short names.

Overall, I'm not sure it's helpful to include the CET_L3 style names as aliases; they are neither canonical in terms of the underlying algorithm by which they were generated (as names like linear_kryw_0_100_c71 are) nor are they easy to remember and distinguish (as names like "fire" are). So I continue to prefer publishing only one short name per colormap except in some special well-justified cases, and that short name should (in the absence of a strong reason otherwise) should be the name the colormap has always had in colorcet.

randallpittman · 2021-02-23T23:25:51Z

Oi, this has gotten complicated.

Ok, I think I want to go back to scratch in another PR. I think my approach here has muddied things too much. Here's the new approach I propose:

All CETperceptual CSV files be "git mv" renamed to their algorithmic name and merged into a single folder. I made a script that counted 37 of the CET-*.csv files being redundant with the "v1" CSV files. After this, mapping in CET_to_py.py is no longer necessary.
I'll rework make_csvs_from_colorcet.m to just generate new CSVs for colormaps we don't already have. These will be named with the algorithmic name found in colorcet.m.
Regarding aliases, I think there's an argument to be made for keeping the CET-* names, and that being that these are the shorthand names most used by Peter at colorcet.com in the gallery and user guide. Beyond those, however, I think it's fine to keep just one "colloquial" alias, and make sure it points to the same thing it has always pointed to.

jbednar · 2021-02-24T01:14:46Z

Ok, I think I want to go back to scratch in another PR. I think my approach here has muddied things too much. Here's the new approach I propose:

Whatever you think is best, since you're doing the work! :-)

All CETperceptual CSV files be "git mv" renamed to their algorithmic name and merged into a single folder. I made a script that counted 37 of the CET-*.csv files being redundant with the "v1" CSV files. After this, mapping in CET_to_py.py is no longer necessary.

That makes sense, and should probably have been done for v2 originally. Alas!

I'll rework make_csvs_from_colorcet.m to just generate new CSVs for colormaps we don't already have. These will be named with the algorithmic name found in colorcet.m.

Ok

Regarding aliases, I think there's an argument to be made for keeping the CET-* names, and that being that these are the shorthand names most used by Peter at colorcet.com in the gallery and user guide. Beyond those, however, I think it's fine to keep just one "colloquial" alias, and make sure it points to the same thing it has always pointed to.

I think that's an argument for documenting that mapping online, so that colorcet users can crossreference with Peter's work. But I'm concerned with the cost to what I think is a larger group of users who just want to use these in Python and aren't concerned with how the maps appear in other docs or in other languages. I'd favor keeping things simple for the casual Python users, while documenting the mapping somewhere (e.g. in a lookup dictionary we publish that translates the CET-* name to the underlying algorithmic name) rather than making the CET names appear everywhere.

randallpittman added 18 commits February 19, 2021 11:00

Fix mixed 4/2 indentation

315945b

(minor) whitespace and a few comments

75becb6

Implement holoviz#60 - Allow multiple aliases

5b2c693

aliases values are lists instead of tuples

0b95135

New octave/MATLAB and python scripts to get new colormaps and aliases…

2e44024

… from colorcet.m

Add missing commas and quotes around string values

199b77a

CET_merge.py - fix merging of dict of lists, and fix mapping sort

b50dc2e

CET_merge.py - keep old aliases if they conflict with new.

adf04e8

Update colormaps and aliases v3 - includes gouldian (CET-L20) parula …

e8d872e

…replacement

Update tests for v3 maps and aliases

fb1b7dd

README for updating colormaps from colorcet.m.

8157cfe

restore # mpl comments

3703864

clarifications to README_assets.md

06aea0c

Restore missing updates to colormaps.

83e296f

replacing missing change in get_aliases

616b7ee

Fix problem with aliases for mapping change conflict

1ce34f8

Fix tests for latest changes

fdb9fc4

Update conflict reporting in CET_merge.py

63890bd

randallpittman marked this pull request as ready for review February 22, 2021 22:11

randallpittman mentioned this pull request Feb 22, 2021

--deleteme-- #62

Closed

jbednar reviewed Feb 23, 2021

View reviewed changes

jbednar added 2 commits February 23, 2021 15:55

Updated with current output

b697b16

Fixed axis linking and balanced columns

0951b31

randallpittman closed this Feb 23, 2021

randallpittman mentioned this pull request Feb 24, 2021

Rename CET maps to algorithmic names #64

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Resolve #60, #56 - Allow multiple aliases, update colormaps from colorcet.m (second attempt) #63

Resolve #60, #56 - Allow multiple aliases, update colormaps from colorcet.m (second attempt) #63

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

		@@ -0,0 +1,100 @@
		# CET_merge.py - create updated `aliases` and `mapping` for CET_to_py.py

Resolve #60, #56 - Allow multiple aliases, update colormaps from colorcet.m (second attempt) #63

Resolve #60, #56 - Allow multiple aliases, update colormaps from colorcet.m (second attempt) #63

Uh oh!

Conversation

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!