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

Segments #3532

Merged
merged 16 commits into from
Oct 2, 2019
Merged

Segments #3532

merged 16 commits into from
Oct 2, 2019

Conversation

poplarShift
Copy link
Collaborator

A quick first draft (sufficient for my own use case, which is why I went ahead and put something together). In particular, I haven't yet thought about how to best implement a variable number of kdims in the bokeh backend which is less important to me at this point but which @philippjfr mentioned should be implemented.

Addresses #3498

screen shot 2019-02-27 at 11 28 23 am

screen shot 2019-02-27 at 11 28 15 am

@philippjfr
Copy link
Member

Very neat, while I did suggest that we could/should support multiple segments per row that could be another element entirely as Segments definitely indicates to me that it's one segment per row.

@philippjfr philippjfr added this to the v1.12.0 milestone Feb 27, 2019
@philippjfr philippjfr added the type: feature A major new feature label Feb 27, 2019
@poplarShift poplarShift changed the title [ENH] Segments Segments Feb 27, 2019
@poplarShift
Copy link
Collaborator Author

Indeed, that could be something like Lines? Although I'd be weary of code duplication.

@poplarShift
Copy link
Collaborator Author

Ok, all your requested changes should have been pushed now.

@philippjfr
Copy link
Member

Thanks, before merging we still need reference gallery entries in examples/reference/element/bokeh/Segment.ipynb and examples/reference/element/matplotlib/Segment.ipynb, but I'm happy to help out on those. They just need to describe what this element represents how to use it in a simple case and a more complex case.

Also if you can think of any examples in the main gallery that could be converted to Segments that might be nice too.

@philippjfr
Copy link
Member

Sorry for the two-stage review, I was short on time earlier.

@poplarShift
Copy link
Collaborator Author

poplarShift commented Feb 28, 2019

I also added example notebooks. I don't think there was any data in the holoviews repo that represented time periods so I just made some mock data, plus some fractal stuff to illustrate more advanced (?) styling.

@philippjfr
Copy link
Member

Looking great. If you don't mind I'll work on a datashader operation which rasterizes segments efficiently and push it to the PR.

@poplarShift
Copy link
Collaborator Author

Great, go ahead!

@philippjfr philippjfr modified the milestones: v1.12.0, v1.12.x Apr 22, 2019
@philippjfr
Copy link
Member

Sorry this has sat around for so long, I'll try to get this in soon.

@philippjfr
Copy link
Member

Just rebased the PR and added datashading support. Just needs some tests now.

@poplarShift
Copy link
Collaborator Author

Ah, great!

@philippjfr
Copy link
Member

Sorry this took so long!

@poplarShift
Copy link
Collaborator Author

No worries, I'm just happy having contributed something!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature A major new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants