Skip to content
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
Merged

Added bokeh VectorField plot #1196

merged 4 commits into from
Mar 13, 2017

Conversation

philippjfr
Copy link
Member

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

@jlstevens
Copy link
Contributor

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
Copy link
Member Author

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
Copy link
Contributor

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
Copy link
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
Copy link
Member Author

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
Copy link
Contributor

... 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
Copy link
Member Author

... 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
Copy link
Member Author

Now with arrowheads:

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

@jlstevens
Copy link
Contributor

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
Copy link
Member Author

Ready to merge.

@jlstevens
Copy link
Contributor

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
@jbednar jbednar deleted the vectorfield_bokeh branch March 13, 2017 20:06
@jbednar
Copy link
Member

jbednar commented Mar 13, 2017

Excellent! Thanks for doing this!

@philippjfr
Copy link
Member Author

philippjfr commented Mar 13, 2017

Now Arrow is really bugging me though:

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

@jbednar
Copy link
Member

jbednar commented Mar 13, 2017

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

@jbednar
Copy link
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
Copy link
Member Author

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

@jbednar
Copy link
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
Copy link
Member Author

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
Copy link
Contributor

Probably ought to move Chart3D to the end of that list

I concur.

@jbednar
Copy link
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
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants