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 bokeh VectorField plot #1196

Merged
merged 4 commits into from Mar 13, 2017

Conversation

Projects
None yet
3 participants
@philippjfr
Member

philippjfr commented Mar 12, 2017

This completes compatibility for all chart plots, that only leaves the Arrow annotation and 3D Elements.

@jlstevens

This comment has been minimized.

Member

jlstevens commented Mar 12, 2017

Fantastic!

Would it be possible to update the website so I can have a look at what the new bokeh plot looks like? I'm still seeing the old version:

image

I can merge before then if you need it.

@philippjfr

This comment has been minimized.

Member

philippjfr commented Mar 12, 2017

You could trigger a rebuild, but for simplicities sake, here's a side-by-side:

screen shot 2017-03-12 at 1 27 56 pm

screen shot 2017-03-12 at 1 28 42 pm

I am cheating a little bit because I've not found a way to add arrows to the vectors, so really it's showing orientation rather than direction. Better than nothing though, unless you think that's a dealbreaker.

@jlstevens

This comment has been minimized.

Member

jlstevens commented Mar 12, 2017

Personally, I think the direction (and not just the angle) is a pretty important part of the semantics of 'vector'. I'm now undecided, @jbednar you have lots of experience with both orientation/vector fields..what do you think?

@jbednar

This comment has been minimized.

Member

jbednar commented Mar 12, 2017

I think it's better to have this than not to, so I'm happy to see it merged. But yes, I think having some way to figure out which end is which is a dealbreaker for the majority of uses of this plot. Can you file an issue with Bokeh about how to do the arrows, both in this case and for the Arrow annotation? I'm not sure what the underlying problem is.

@philippjfr

This comment has been minimized.

Member

philippjfr commented Mar 12, 2017

both in this case and for the Arrow annotation?

I'll try some ideas I have on this implementation, but bokeh's Arrow annotation is perfectly well defined, the issue is that our Arrow Element is very limited in what it can do and should be changed, see #34.

@jlstevens

This comment has been minimized.

Member

jlstevens commented Mar 12, 2017

... is that our Arrow Element is very limited in what it can do

This is mainly because the holoviews Arrow was only designed as an annotation element i.e to point on things on a plot with a label. I don't think the Arrow element should be used to define vector fields, but it would be ok if the bokeh VectorFieldPlot class uses bokeh arrow annotations.

@philippjfr

This comment has been minimized.

Member

philippjfr commented Mar 12, 2017

... is that our Arrow Element is very limited in what it can do

Sorry wasn't clear, what I meant is that our arrow element only has an x, y location and a direction but what we really need is for it to specify the start and end points of the arrow so you can have arbitrary directions. Nothing to do with vector fields.

@philippjfr

This comment has been minimized.

Member

philippjfr commented Mar 13, 2017

Now with arrowheads:

screen shot 2017-03-13 at 12 02 46 am

@jlstevens

This comment has been minimized.

Member

jlstevens commented Mar 13, 2017

Great!

Looks like it was a bit more effort to get the arrows positioned properly (using 'segment' instead of 'ray') but I do think supporting arrows properly is worth it.

@philippjfr

This comment has been minimized.

Member

philippjfr commented Mar 13, 2017

Ready to merge.

@jlstevens

This comment has been minimized.

Member

jlstevens commented Mar 13, 2017

Tests are green. I'm glad I can merge it with proper arrow support - thanks!

@jlstevens jlstevens merged commit 8484dca into master Mar 13, 2017

4 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.07%) to 78.569%
Details
s3-reference-data-cache Test data is cached.
Details

@jbednar jbednar deleted the vectorfield_bokeh branch Mar 13, 2017

@jbednar

This comment has been minimized.

Member

jbednar commented Mar 13, 2017

Excellent! Thanks for doing this!

@philippjfr

This comment has been minimized.

Member

philippjfr commented Mar 13, 2017

Now Arrow is really bugging me though:

screen shot 2017-03-13 at 8 09 17 pm

@jbednar

This comment has been minimized.

Member

jbednar commented Mar 13, 2017

It does stand out there, doesn't it? :-)

@jbednar

This comment has been minimized.

Member

jbednar commented Mar 13, 2017

Probably ought to move Chart3D to the end of that list, btw; it's not the first thing people need, out of those items, and it's demotivating to see something unsupported as anything but the last thing in a list.

@philippjfr

This comment has been minimized.

Member

philippjfr commented Mar 13, 2017

Agreed, although I just updated the image, I'd left out all the charts.

@jbednar

This comment has been minimized.

Member

jbednar commented Mar 13, 2017

Right; I figured that out and updated my own comment to match. :-) 3D elements are definitely their own separate thing, not being easily overlaid like the rest are, so it makes sense in many ways to have them at the end.

@philippjfr

This comment has been minimized.

Member

philippjfr commented Mar 13, 2017

3D elements are definitely their own separate thing, not being easily overlaid like the rest are, so it makes sense in many ways to have them at the end.

3D Elements are overlayable with other 3D Elements, and at least in matplotlib even 2D Elements, where the z-coordinate defaults to 0.

@jlstevens

This comment has been minimized.

Member

jlstevens commented Mar 13, 2017

Probably ought to move Chart3D to the end of that list

I concur.

@jbednar

This comment has been minimized.

Member

jbednar commented Mar 14, 2017

Being able to overlay 2D Elements is cool; that should be mentioned at the start of the Charts3D section.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment