arviz icon indicating copy to clipboard operation
arviz copied to clipboard

Bokeh, transform all colors to hex

Open ahartikainen opened this issue 5 years ago • 24 comments

Input all colors to bokeh functions as hex.

We can use matplotlib helper functions to transform special syntax

color="C0"

etc

ahartikainen avatar Dec 08 '19 22:12 ahartikainen

@OriolAbril can I try fixing it?

mrinalnilotpal avatar Jan 13 '20 02:01 mrinalnilotpal

@mrinalnilotpal Definitely!

OriolAbril avatar Jan 13 '20 11:01 OriolAbril

can anybody tell me which files I should look at in order to solve the issue?

animesh-007 avatar Jan 21 '20 15:01 animesh-007

I think you would need to create a function that transforms certain colors (rgb, 'C{i}' syntax etc to hex)

These are for bokeh plots

ahartikainen avatar Jan 21 '20 16:01 ahartikainen

matplotlib have some functions for color conversion. https://matplotlib.org/api/colors_api.html#functions and the "CN" colors can be accessed from matplotlib.rcParams['axes.prop_cycle']

aloctavodia avatar Jan 21 '20 18:01 aloctavodia

Is this issue still to be resolved?

sahajk21 avatar Feb 08 '20 05:02 sahajk21

@mrinalnilotpal @animesh-007 @sahajsk21 It is still pending. Just to make sure it is clear what this PR entails, I'll try to extend the description. The goal is to have some easy way to convert all kinds of colors used by matplotlib (such as C0, C1 to refer to default colors in prop_cycle, named colors, single letter color abbreviations, rgb...) to hex in order to use them in bokeh. Then, modify the code where necessary to make both backends plot the same colors (which is currently not the case, e.g. matplotlib's ppc vs bokeh's ppc

It looks like to_hex is exactly what we are looking for. I think a way to tackle this would be to convert all color references to hex before passing them to the backend files. Then both backends should plot the same colors without much hassle.

OriolAbril avatar Feb 09 '20 17:02 OriolAbril

Thanks @OriolAbril I'll be working to solve this issue

sahajk21 avatar Feb 10 '20 00:02 sahajk21

@OriolAbril I think this issue has been inactive for a while. Could I work on it?

amukh18 avatar Feb 18 '20 17:02 amukh18

@sahajsk21 do you still plan to work on this?

OriolAbril avatar Feb 18 '20 18:02 OriolAbril

@amukh18 @OriolAbril I have been working on issue so he can take up the issue.Thanks

sahajk21 avatar Feb 19 '20 00:02 sahajk21

I have been working on issue so he can't take up the issue.Thanks

Could there be a typo so the message is this? @sahajsk21 There is no rush, work at your own pace, this is why we ask :).

Sorry if my previous wording sounded harsh, it wasn't the intention. Also, please let us know if you were to need any help

OriolAbril avatar Feb 19 '20 00:02 OriolAbril

@OriolAbril No it wasn't a typo. Don't worry, I didn't take the comment in the negative sense. Actually, I have been going through the code and making myself familiar first before working on the issue. That is why it is taking so long. But I think @amukh18 can still take up this issue.

@amukh18 @OriolAbril I have been working on another issue so he can take up the issue. Thanks

sahajk21 avatar Feb 19 '20 02:02 sahajk21

@sahajsk21 @OriolAbril Thank you. I'll begin working on it right away.

amukh18 avatar Feb 19 '20 16:02 amukh18

@OriolAbril I have a question. If I were to add the to_hex function to plot_khat as follows:

if hlines_kwargs is None:
        hlines_kwargs = {}
    hlines_kwargs.setdefault("linestyle", [":", "-.", "--", "-"])
    hlines_kwargs.setdefault("alpha", 0.7)
    hlines_kwargs.setdefault("zorder", -1)
    hlines_kwargs.setdefault("color", to_hex("C1"))

    if coords is None:
        coords = {}

    if color is None:
        color = "C0"
    color = to_hex(color)

Is this supposed to return any errors?

amukh18 avatar Feb 21 '20 10:02 amukh18

I think this should work for this case

OriolAbril avatar Feb 21 '20 16:02 OriolAbril

Hmm, pytest returned some errors. I will see what the unit tests on the pipeline say on committing.

amukh18 avatar Feb 21 '20 17:02 amukh18

There is now a converter function available (added by #1084), however, is is not used in all plots yet. I am reopening the issue as a remainder to use the converter in all plots

OriolAbril avatar May 10 '20 19:05 OriolAbril

~plot_hdi is a great candidate to take advantage of this: bokeh code already uses matplotlib cycle:~

EDIT: I updated the setting of defaults in #1241 to reduce these lines below:

https://github.com/arviz-devs/arviz/blob/4fbd1e2a3fa6c9b9054c0aacbc2f547b11754662/arviz/plots/backends/bokeh/hdiplot.py#L24-L32

To a single line when setting the defaults in plot_kwargs and fill_kwargs


Plenty (if not all) functions could also take advantage of this which will give both matplotlib and bokeh plots the same style and colors :art:

OriolAbril avatar Jun 17 '20 15:06 OriolAbril

@OriolAbril would like to work on this. Thanks

Rishabh261998 avatar Mar 09 '21 21:03 Rishabh261998

I have assigned it to you. I have to confess I have not followed this for a while so I don't really know which plots use this and which don't, you'll have to first check where it's needed and then make the changes.

OriolAbril avatar Mar 10 '21 08:03 OriolAbril

@OriolAbril Is this issue still open?

ch1booze avatar Oct 03 '22 12:10 ch1booze

If so, I would like to be assigned the task. Pending your reply, I am getting acquainted with the codebase.

ch1booze avatar Oct 03 '22 12:10 ch1booze

@OriolAbril Upon cloning and reviewing the code, it seems as if the refactoring needed has been done by @Rishabh261998. I checked if he made any PRs regarding this issue, but I didn't see any or maybe I am looking at the whole PR thing wrong. I am still in search of my first Open Source contribution😋.

ch1booze avatar Oct 03 '22 14:10 ch1booze

import itertools
import matplotlib.pyplot as plt

def process_color(color, plot_kwargs):
"""Process color input and set line color in plot_kwargs."""
if color[0] == "C":
# Use color cycle from matplotlib
color_cycle = itertools.cycle(plt.rcParams["axes.prop_cycle"].by_key()["color"])
line_color = next(itertools.islice(color_cycle, int(color[1:]), None))
else:
line_color = color
plot_kwargs.setdefault("line_color", line_color)

Alternative updated code for the commit above https://github.com/arviz-devs/arviz/blob/4fbd1e2a3fa6c9b9054c0aacbc2f547b11754662/arviz/plots/backends/bokeh/hdiplot.py#L24-L32

Dhruv3sood avatar Mar 27 '23 20:03 Dhruv3sood

Is this issue still open?

jasochen7 avatar Mar 29 '23 03:03 jasochen7

It should have been closed already @jasochen7, closing now.

OriolAbril avatar Mar 29 '23 07:03 OriolAbril