-
-
Notifications
You must be signed in to change notification settings - Fork 366
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
Support antialiasing in pipeline API #1213
Conversation
@@ -431,7 +431,6 @@ def line(self, source, x=None, y=None, agg=None, axis=0, geometry=None, | |||
line_width = 0 | |||
|
|||
glyph.set_line_width(line_width) | |||
glyph.antialiased = (line_width > 0) |
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.
This is now handled in line.py
.
@@ -61,6 +61,8 @@ class _AntiAliasedLine(object): | |||
|
|||
def set_line_width(self, line_width): | |||
self._line_width = line_width | |||
if hasattr(self, "antialiased"): |
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.
This isn't excellent, but is the simplest solution available given that the Glyph
class has the antialiased
attribute and this _AntiAliasedLine
class is a mixin for line glyphs only. In fact the antialiased
flag is always available here given how we are using it, so this hasattr
isn't really needed but it is good practice to check before setting it.
In the long run, when we add antialiasing to other glyphs such as points it would be better to remove the _AntialisedLine
class and move the line_width
to the Glyph
base class. But it will need a more generic name than line_width
(antialiased_spread
?) which is TBD.
Codecov Report
@@ Coverage Diff @@
## main #1213 +/- ##
==========================================
+ Coverage 84.63% 84.65% +0.02%
==========================================
Files 35 35
Lines 8354 8355 +1
==========================================
+ Hits 7070 7073 +3
+ Misses 1284 1282 -2
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
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.
Thanks!
Fixes #1204.
In the
pipeline
API we weren't passing through theantialias
flag. With this fix, the OP's example works fine for all values ofline_width
: