Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Color maps consistency and convenient access #205

Closed
tgandor opened this issue Feb 11, 2020 · 3 comments
Closed

Color maps consistency and convenient access #205

tgandor opened this issue Feb 11, 2020 · 3 comments

Comments

@tgandor
Copy link
Contributor

tgandor commented Feb 11, 2020

There are a few minor issues with color maps (or colormaps?) as we have them right now:

  • default_colormap as an argument value is very verbose in documents or signature hints in IDEs
  • some colormaps imported from paraview run from attribute -1 to 1 instead of 0 to 1
  • matplotlib has a nice way of specifying colormaps by string name
  • we have a few versions of same colormaps in basic, MPL and paraview
  • some imported colormaps are redundant, i.e. have repeated colors for different adjacent attribute values (many times over)
  • maybe the precision is too big in some constants (it gets mapped to 0..255 RGB anyway)
  • also why do we need to do np.array(k3d.colormaps.some_group.SomeColorMap, dtype=np.float32) for our own built-in colormaps? Couldn't we have them ready to use, as ndarrays with the correct dtype?

This issue is for answering and addressing these questions.

@tgandor
Copy link
Contributor Author

tgandor commented Feb 11, 2020

One more thing to add to this list (might change API):

  • why do we call the colors for Voxels color_map? It's a list of colors, so class_colors might be a better name; also it has very different properties to color maps found elswhere (int instead of float32, and also needs not be divisible by 4, and have the values in range 0..1)

@artur-trzesiok
Copy link
Collaborator

artur-trzesiok commented Feb 11, 2020

@tgandor thanks for sharing.

default_colormap as an argument value is very verbose in documents or signature hints in IDEs

That's right. We have to do something about it for the docs.

some colormaps imported from paraview run from attribute -1 to 1 instead of 0 to 1

It's not a problem. Colormap definitions do not assume a value scale. Everything is projected into the color_range. This is the standard for defining colormap in paraview (source: https://gitlab.kitware.com/paraview/paraview/raw/master/Remoting/Views/ColorMaps.json )

matplotlib has a nice way of specifying colormaps by string name

colormap resolving is on kernel side in K3D. I don't want to increase the size of snapshots/k3djs codebase to hold every potential colormap. So we can only implement setter to automagically replace string to a proper array. but:

obj = k3d.surface(...., color_map='viridis')
assert(obj.color_map == 'viridis)

Will fail. I don't think that it's worth introducing that.

we have a few versions of same colormaps in basic, MPL and paraview

What's the problem? Some k3d users come from the world of matplotlib, others from the world of VTK. The collections are separated, but complete and original. Do we want to save a few kb in the source code?

some imported colormaps are redundant, i.e. have repeated colors for different adjacent attribute values (many times over)

Yes. For example for those colormap:
image

maybe the precision is too big in some constants (it gets mapped to 0..255 RGB anyway)

Finally, the colormap is transferred to the frontend anyway as binary float32 data. It doesn't matter what you write in the source code. The bigger value for us is that we have k3d.matplotlib_color_maps rather than k3d.matplotlib_like_color_maps

Please look at https://github.com/matplotlib/matplotlib/blob/master/lib/matplotlib/_cm.py and https://github.com/matplotlib/matplotlib/blob/master/lib/matplotlib/_cm_listed.py

K3D use name 'matplotlib' and 'paraview' so I don't think that we have credibility to affect the data.

also why do we need to do np.array(k3d.colormaps.some_group.SomeColorMap, dtype=np.float32) for our own built-in colormaps? Couldn't we have them ready to use, as ndarrays with the correct dtype?

It's a fully automated process in object constructor , right? Of course We can change it.

why do we call the colors for Voxels color_map? It's a list of colors, so class_colors might be a better name; also it has very different properties to color maps found elswhere (int instead of float32, and also needs not be divisible by 4, and have the values in range 0..1)

class_colors sound good but on the other hand isn't it is a colormap in fact? That change will introduce a breaking change to API so @marcinofulus should decide.

@tgandor
Copy link
Contributor Author

tgandor commented Feb 12, 2020

Some conclusions after discussion:

  • use hacks for autodoc Keeping original signatures for functions/methods sphinx-doc/sphinx#759 to have sensible signatures
  • color maps as NumPy arrays: OK, construction is 10x slower (450 us instead of 45), but it only happens in import anyway (my comment: and we would pay it anyway when using color maps)
  • matplotlib import: min_samples is suspicious; may be removed because of oversampling
  • specifying 'by string' - rejected. (we might add some fancy error message like 'pass array of shape (4k,) or use a built in map from k3d.colormaps.{basic, MPL, paraview} modules'.

Rest of stuff, as class_colors is to be discussed separately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants