-
-
Notifications
You must be signed in to change notification settings - Fork 402
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
Add color to bars for Plotly #6294
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6294 +/- ##
==========================================
- Coverage 88.55% 88.50% -0.06%
==========================================
Files 323 323
Lines 68000 68079 +79
==========================================
+ Hits 60218 60250 +32
- Misses 7782 7829 +47 ☔ View full report in Codecov by Sentry. |
holoviews/plotting/plotly/chart.py
Outdated
@@ -280,6 +281,10 @@ def get_data(self, element, ranges, style, **kwargs): | |||
for d in (xdim, group_dim)], | |||
y: np.nan_to_num(values)}) | |||
|
|||
if self.color: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a better method to put this in?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think making color
a plot option is the right approach here. I've tried something else that seemed to work, it'd need some more testing and unit tests.
I'm also not sure what convention HoloViews follows on naming style options, does it attempt to standardize them across backends or does it instead try to stay closer to the backend naming scheme?
diff --git a/holoviews/plotting/plotly/chart.py b/holoviews/plotting/plotly/chart.py
index a7a831bbd..bcb7c82ee 100644
--- a/holoviews/plotting/plotly/chart.py
+++ b/holoviews/plotting/plotly/chart.py
@@ -198,9 +198,7 @@ class BarPlot(BarsMixin, ElementPlot):
show_legend = param.Boolean(default=True, doc="""
Whether to show legend for the plot.""")
- color = param.String(default=None, doc="The color of the bars.")
-
- style_opts = ['visible']
+ style_opts = ['visible', 'color']
selection_display = PlotlyOverlaySelectionDisplay()
@@ -281,12 +279,14 @@ class BarPlot(BarsMixin, ElementPlot):
for d in (xdim, group_dim)],
y: np.nan_to_num(values)})
- if self.color:
- for bar in bars:
- bar['marker_color'] = self.color
-
return bars
+ def graph_options(self, element, ranges, style, **kwargs):
+ if 'color' in style:
+ style['marker_color'] = style.pop('color')
+ opts = super().graph_options(element, ranges, style, **kwargs)
+ return opts
+
def init_layout(self, key, element, ranges, **kwargs):
layout = super().init_layout(key, element, ranges)
stack_dim = None
@hoxbro in fact you already added a test which I didn't need to touch, so we're good. |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Fixes #5658