-
-
Notifications
You must be signed in to change notification settings - Fork 403
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
Added warning when datashading with multi_y=True #5829
base: main
Are you sure you want to change the base?
Conversation
Note that I am hardcoding 'bokeh' as the backend: |
Codecov Report
@@ Coverage Diff @@
## main #5829 +/- ##
==========================================
- Coverage 88.23% 88.23% -0.01%
==========================================
Files 309 309
Lines 63866 63872 +6
==========================================
+ Hits 56354 56359 +5
- Misses 7512 7513 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
def __call__(self, element, **kwargs): | ||
opts = element.opts.get('plot', 'bokeh') | ||
opts = {} if opts is None else opts.kwargs | ||
if 'multi_y' in opts: |
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.
if 'multi_y' in opts: | |
if 'multi_y' in opts and isinstance(element, CompositeOverlay): |
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.
Not sure that's quite right, but I don't see a good reason to warn if you just provide an Element. Also the multi_y
may be set after the fact and then this warning won't do anything. So overall I don't think this approach is very good.
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.
Fair enough. Where would you warn from when using datashader with multi_y=True
then?
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.
One other alternative of course is to not warn at all and to document this instead...
Simple PR to address #5823
Now that
RangeXY
is extra axis aware, you still need to overlay rasterize calls to datashade withmulti_y=True
. This PR adds such a warning/suggestion for the user until we can support aRangeXMultiY
stream (or similar).