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

Grouped bars #59

Closed
wants to merge 4 commits into from
Closed

Grouped bars #59

wants to merge 4 commits into from

Conversation

nbedi
Copy link
Member

@nbedi nbedi commented May 26, 2016

Quick attempt at grouped bars. It currently manually sets an Ordinal scale which isn't great. Let me know what you think/what I should change (and if you have suggestions for the scale). Adding tests soon.

Should be able to do stacked bars and grouped/stacked columns after this gets set.

grouped

@onyxfish
Copy link
Collaborator

Wow! I wasn't expecting to get a PR for anything like this. I think it looks great, however, my big concern with this is the input format of the data series. This format...

data = [
    (3, 'Hello'),
    (5, 'Hello'),
    (5, 'Hello'),
    (5, 'How'),
    (5, 'How'),
    (7, 'Are'),
    (9, 'Are'),
    (4, 'You'),
    (3, 'You'),
    (2, 'You')
]

...seems like an unlikely way to find data organized for charting. In particular, it provides no indication that there are 3 separate "series" here.

I think there are some larger architectural issues around multi-dimensional data that need I need to think through before we tackle these chart types further.

@nbedi
Copy link
Member Author

nbedi commented May 26, 2016

👍 agreed. My first attempt tried to do something multi-dimensional but that got way too messy since the structure doesn't exist yet. Had an urge to try anyway though...

@onyxfish
Copy link
Collaborator

Thought about this while I was rowing. Here's what I'm thinking: Broadly, at a logical level, I don't want to make leather overly-generic. I want it really tightly focused on the 80% of use-cases that everyone has. To that end, I think we do this:

  • Add GroupedSeries class. The default constructor is def __init__(self, data, x=None, y=none, group=None, ...). The default data format is (x, y, group). More complicated data can be parsed using accessor functions, like the existing Series class does.
  • Add GroupedBars and GroupedColumns shape subclasses that render GroupedSeries data.
  • If a GroupSeries is added to a chart (e.g. add_grouped_bars(data, ...)) then it's the only series that chart can have. You can't layer regular Series on top. We should probably have error checking in Chart.add_series for this.
  • If a chart has a GroupedSeries then Legend renders the sub-series names. (GroupedSeries itself won't even need a name param.)
  • StackedBars and StackedColumns should be able to cleanly repurpose GroupedSeries.
  • I'm not sure how we handle colors...

This is a pretty complicated thing to map out entirely in my head, so I'm probably missing things, but how does that sound?

@nbedi
Copy link
Member Author

nbedi commented May 27, 2016

Sounds good! Definitely helps establish a tighter structure, I was originally trying to be clever/generic by allowing some sort of Series of Series and then magically rendering it but that was a mess.

I'm only wrestling a bit with the third point. My gut doesn't want it to be a limitation but the use cases may fall in the 20% leather won't cover. I'm also not 100% familiar with the codebase yet so I may be missing an absolute block on allowing a Series on top of a GroupedSeries.

@onyxfish
Copy link
Collaborator

Yeah nested Series was my first inclination too, but even if we could make it work technically it really makes the APIs a lot harder for basic cases, I think.

Regarding the layering, if there is a way to make it work, I'm not against it or anything. The big blocker I foresee is how the Legend would render if you have both Series and "groups" that need to be labelled. Unless you can think of an elegant solution I'm inclined to implement the easy case and then see if we can work backwards to the harder one.

One more thing to keep in mind: this should work with Lattice too. (Grids of stacked columns seem useful!) However the legend rendering may be funny in that case too. (We probably don't want n legends.)

Of course, we don't have to solve all of these things at once. Like you said, let's put down a foundation. That may cause more general solutions to present themselves.

@onyxfish
Copy link
Collaborator

onyxfish commented Jun 8, 2016

@nbedi FYI, I believe the new CategorySeries (open to better ideas for the name...) should make implementing Grouped/Stacked Bars/Columns easy at this point. They just need to be implemented as new shape types. Lemme know if you feel like taking a crack at this. #25 #26

@nbedi
Copy link
Member Author

nbedi commented Jun 8, 2016

👍 will try it out soon

@nbedi
Copy link
Member Author

nbedi commented Jun 9, 2016

moved to #70

@nbedi nbedi closed this Jun 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants