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

Stacked area operation #1193

Merged
merged 5 commits into from Apr 14, 2017

Conversation

Projects
None yet
3 participants
@philippjfr
Member

philippjfr commented Mar 11, 2017

Added a very simple operation that lets you produce a stacked area chart from an overlay of areas. Example included in the holoviews-contrib gallery PR.

@jlstevens

This comment has been minimized.

Member

jlstevens commented Mar 12, 2017

Simple and useful as it is, I want to resist adding more operations unless they are really well justified. A dedicated stacked_area notebook in contrib would help convince me :-)

I'm now thinking of adding an operations directory to contrib where each operation has a dedicated notebook there. We should do this for all our existing operations and make it a requirement before adding new ones.

@philippjfr

This comment has been minimized.

Member

philippjfr commented Mar 12, 2017

A dedicated stacked_area notebook in contrib would help convince me :-)

There is an example in the contrib gallery PR. I wouldn't mind having an operation section for the gallery but I'd like to avoid fragmenting contrib into a bunch of separate folders. Maybe its worth discussing the organization again. Currently in the main repo we have Tutorials, which are long and Examples, which are really case studies. Then contrib has quickstart notebooks, which I would like to morph into a user guide and my new PR adds gallery examples, which should be self-contained examples on how to construct a single plot. I think operation documentation could fit into both the quickstart and the gallery example format. Just have to decide.

@jlstevens

This comment has been minimized.

Member

jlstevens commented Mar 12, 2017

I'm open to re-organizing contrib and we should discuss it. My main point for this PR is I would like all our operations to have some dedicated documentation/examples in notebook format explicitly about them.

@philippjfr

This comment has been minimized.

Member

philippjfr commented Mar 12, 2017

And my point was that there is such an example already, and I'm asking whether you think a "user guide"/"quickstart" format or a simply gallery example format would be preferable for these.

@jlstevens

This comment has been minimized.

Member

jlstevens commented Mar 12, 2017

I'm not talking about one example - each operation needs several examples to be justified. A notebook with three examples would be reasonable.

@philippjfr

This comment has been minimized.

Member

philippjfr commented Mar 12, 2017

I'm not talking about one example - each operation needs several examples to be justified. A notebook with three examples would be reasonable.

Not much point in several examples for this operation and maybe others, it does one simple thing. I think operations are so well contained that gallery examples are probably fine, could just break the gallery down into multiple sections like matplotlib does: http://matplotlib.org/gallery.html

@jbednar

This comment has been minimized.

Member

jbednar commented Mar 12, 2017

I think we should discuss possible re-organizations for the website/tutorials/examples/quickstarts/contrib very soon, including how each of these relate to the underlying backends, so that people can have a better experience finding what exists and how to use it.

In this particular case, where I think people are most likely to realize they need this operation is in the Elements tutorial (or whatever happens to the contents of that) -- it's a handy way to build a stacked area, and should be introduced or at least linked from wherever we introduce a stacked area chart.

Personally, I love operations and wish we had more of them; they really exploit the power of HoloViews to keep track of all the metadata as the data is transformed. It's just going to be difficult to keep track of all of them! In this case, it's essentially a constructor for a stacked area, so it has a natural home; others will be less clear how to find.

@jlstevens

This comment has been minimized.

Member

jlstevens commented Mar 12, 2017

Given this operation is closely associated with Area, I would be open to having this operation available as a classmethod (as an alternate constructor). We have some of these already, e.g to load an RGB from file. If we did that, then it would make perfect sense to introduce it in the Elements tutorial.

Personally, I love operations and wish we had more of them; they really exploit the power of HoloViews to keep track of all the metadata as the data is transformed. It's just going to be difficult to keep track of all of them!

I agree that operations are very valuable and yes, my issue with them is just that a scheme for keeping them organized in a sensible way is not at all obvious (this includes giving them sensible names!).

@philippjfr

This comment has been minimized.

Member

philippjfr commented Mar 13, 2017

Given this operation is closely associated with Area, I would be open to having this operation available as a classmethod (as an alternate constructor). We have some of these already, e.g to load an RGB from file. If we did that, then it would make perfect sense to introduce it in the Elements tutorial.

I'd be happy to expose the operation via a classmethod, but I'd keep it as an ElementOperation so you can apply it to a HoloMap/DynamicMap.

@jlstevens

This comment has been minimized.

Member

jlstevens commented Mar 13, 2017

I'd be happy to expose the operation via a classmethod, but I'd keep it as an ElementOperation so you can apply it to a HoloMap/DynamicMap.

Good point. A classmethod alternate constructor wouldn't be a proper demonstration of the operation which is why I still think it needs to be documented independently of elements tutorial.

@philippjfr

This comment has been minimized.

Member

philippjfr commented Mar 13, 2017

I also wouldn't call it an alternative constructor. A single area cannot hold multiple stacked areas. The operation takes an overlay of areas, stacks them, and then outputs a new overlay of the stacked areas.

@jlstevens

This comment has been minimized.

Member

jlstevens commented Mar 13, 2017

I also wouldn't call it an alternative constructor. A single area cannot hold multiple stacked areas.

Good point. I now have no idea where this operation belongs.

@jbednar

This comment has been minimized.

Member

jbednar commented Mar 14, 2017

The "Area between curves" example in Elements already shows how to make a stacked area chart, right? So this is another way of doing what is already illustrated, so it can go there, even though yes, it's not strictly a constructor since there is no separate StackedArea chart (as there shouldn't be, since an Overlay of Areas is sufficient).

@jlstevens

This comment has been minimized.

Member

jlstevens commented Mar 14, 2017

I'm not necessarily opposed to it in this instance. But here are my worries about sticking it in Elements:

  1. If we do it for one such operation, doesn't that elements will be the place to demo all similar operations? There could be a great many of these!
  2. Elements is already really long and I do want it to be a complete reference of all the elements which means we can't spend too long demoing each element.

I suppose my objections go away if we do the proposed refactor of the elements tutorial into lots of little notebooks, one per element. That will be a big job (or at least tedious) so I suppose I do accept this suggestion on the condition that we split up the elements notebook soon! As much as I would like that to happen before 1.7, I'm not sure it will. :-(

@jbednar

This comment has been minimized.

Member

jbednar commented Mar 14, 2017

My personal preference is to replace Elements with a gallery, where each bit is a standalone notebook yet there is a clear high-level overview that is easy to navigate. But that's an orthogonal issue; what we have right now is a combined Elements tutorial, so that's where I'd put the example so that we can merge this and move on.

@philippjfr

This comment has been minimized.

Member

philippjfr commented Mar 14, 2017

My personal preference is to replace Elements with a gallery, where each bit is a standalone notebook yet there is a clear high-level overview that is easy to navigate.

One per Element per backend or have examples for all backends in one notebook?

But that's an orthogonal issue; what we have right now is a combined Elements tutorial, so that's where I'd put the example so that we can merge this and move on.

Agreed, I'll do that later.

@jlstevens

This comment has been minimized.

Member

jlstevens commented Mar 14, 2017

But that's an orthogonal issue; what we have right now is a combined Elements tutorial, so that's where I'd put the example so that we can merge this and move on.

I also agree. I'll merge once that example is added.

@jlstevens

This comment has been minimized.

Member

jlstevens commented Mar 14, 2017

One per Element per backend

I'm tempted to say we should be as granular as possible. People only using bokeh shouldn't need to even see code related to matplotlib in the example (and vice versa). I does mean there will be lots of little notebooks to manage but in some ways that may be easier to manage than one monolithic notebook.

@jbednar

This comment has been minimized.

Member

jbednar commented Mar 14, 2017

Hmm. I guess there could be a badge on each such example to switch between the different supported backends for comparison, and then it would be sticky-- the user would see whichever the last backend selected was, when they look at the next gallery example. That way people can easily compare backends, while someone who cares only about one backend can ignore all the others.

Seems like a lot of tiny files to maintain, though. Do you think we need to make more changes to individual Element sections over time, or more changes to the overall set of examples? If the former, we'd probably want to edit separate files. If the latter, we'd want to consider having large groups of Elements joined together in some larger file that's then split up to make the separate examples online.

@jlstevens

This comment has been minimized.

Member

jlstevens commented Mar 23, 2017

Just a thought - maybe we could host all the tiny files on holoviews-contrib? It would be nice to keep this stuff away from the main codebase in a repo that allows for easier edits.

@philippjfr

This comment has been minimized.

Member

philippjfr commented Mar 23, 2017

Just a thought - maybe we could host all the tiny files on holoviews-contrib? It would be nice to keep this stuff away from the main codebase in a repo that allows for easier edits.

Agreed, personally I'd just split the gallery into multiple sections one of which is devoted entirely to the different elements.

@jbednar

This comment has been minimized.

Member

jbednar commented Mar 23, 2017

I agree, the elements tutorial plots would make a good section in the gallery, along with other sections showing off more complicated examples.

What makes holoviews-contrib have easier edits? Presumably the outputs from whatever we do with the Elements tutorial will need to be tested for regressions in any case, so I'm not sure what moving them between repos will do for our ability to edit them.

@jlstevens

This comment has been minimized.

Member

jlstevens commented Mar 23, 2017

so I'm not sure what moving them between repos will do for our ability to edit them.

Having a repo that doesn't contain core holoviews code makes it easier. We explicitly have a more lax merge policy in contrib.

@philippjfr philippjfr added the feature label Apr 14, 2017

philippjfr added some commits Apr 14, 2017

@philippjfr

This comment has been minimized.

Member

philippjfr commented Apr 14, 2017

@jlstevens Wouldn't mind if you could merge this.

@jlstevens

This comment has been minimized.

Member

jlstevens commented Apr 14, 2017

Looks good. I believe we already have an issue about splitting up the elements tutorial.

Merging.

@jlstevens jlstevens merged commit 4ec6351 into master Apr 14, 2017

0 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/travis-ci/push The Travis CI build is in progress
Details

@philippjfr philippjfr deleted the stack_area branch Apr 19, 2017

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