-
-
Notifications
You must be signed in to change notification settings - Fork 407
fix: Rectangles and Segments color #5014
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
base: main
Are you sure you want to change the base?
fix: Rectangles and Segments color #5014
Conversation
| # Geometries | ||
| options.Rectangles = Options('style', line_color='black') | ||
| options.Rectangles = Options('style', color=Cycle(), cmap=dflt_cmap, line_color='black') | ||
| options.Segments = Options('style', color='black', cmap=dflt_cmap) |
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.
What's the motivation for having one of these be a cycle and the other be a fixed 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.
It looked like all the other "line-only" things like Segments are drawn in black, like error bars. I suppose it depends on the main use case.
For the Rectangles, I suppose it is an oversight when it was added, as the options were set twice with different values. Polygons sets a facecolor as well. OTH, things like Ellipse do not have any filling by default, so maybe Rectangles should also default to border-only in black.
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.
The current behavior is not aligned to anything else I tried though, since it is filled by default with blue. Elements like Ellipse are not filled by default at all.
* Remove duplicated setting of Rectangles options * Make Segments default to black, like all the other "line-only" shapes * Make Rectangles use the default color cycle Fixes holoviz#4981
8d5133b to
d6b0d4b
Compare
|
As I went through my workarounds, I realized this was still open. @jbednar This rebases cleanly on |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5014 +/- ##
=======================================
Coverage 88.79% 88.80%
=======================================
Files 323 323
Lines 68960 68958 -2
=======================================
- Hits 61236 61235 -1
+ Misses 7724 7723 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Fixes #4981