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.

Sign up for GitHub

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

Jump to bottom

[WIP] [Discuss] Make use of all grouped data to draw pie chart #2128

Closed
wants to merge 1 commit into from
Closed

[WIP] [Discuss] Make use of all grouped data to draw pie chart #2128

wants to merge 1 commit into from

Conversation

sravan-s
Copy link
Contributor

What is this PR for?

Now, grouped pie charts only uses the data from the first group
With this fix-

  • Add data from all groups to variable d3g, so all groups could be rendered
  • Rewrite for loop with map and concat
  • Refactor some variables to const and let

What type of PR is it?

[Bug Fix]

What is the Jira issue?

How should this be tested?

  • Create a built in pie chart visualization
  • Select a column to group the data
  • Should display the visualization based on all the available grouped data

Questions:

  • Does the licenses files need update? No
  • Is there breaking changes for older versions? No
  • Does this needs documentation? No

@sravan-s
Make use of all grouped data to draw pie chart
652c943 
* Add data from all groups to variable d3g
* Rewrite for loop with map and concat
* Refactor some variables to const and let
@sravan-s
Copy link
Contributor Author

sravan-s commented Mar 13, 2017 •
edited

As per ZEPPELIN-2237, in situations in which data is grouped, the pie chart only displays the data from first group. With this fix, data from all the groups would be displayed in the pie chart

Discuss

I would prefer to see pie chart with broken slices: https://www.amcharts.com/demos/pie-chart-broken-slices/. Not sure if nvd3 supports this feature. I won't suggest bringing in a new charting library to implement such an idea. Thoughts...

While I was working on this issue, I came across: another one ZEPPELIN-2253, pie chart rendering issue when column selected as 'key' is changed

@sravan-s sravan-s closed this Mar 13, 2017
@sravan-s sravan-s reopened this Mar 13, 2017
@sravan-s
Copy link
Contributor Author

@Leemoonsoo @AhyoungRyu Any comments on this?

@Leemoonsoo
Copy link
Member

LGTM and merge to master and branch-0.7 if no more comment
Thanks @sravan-s for the fix!

@sravan-s
Copy link
Contributor Author

@Leemoonsoo Please review #2132 . I came across it while working on this. I believe it should be hadled first

@asfgit asfgit closed this in a2cd4ae Mar 15, 2017
asfgit pushed a commit that referenced this pull request Mar 15, 2017
@sravan-s @Leemoonsoo
[WIP] [Discuss] Make use of all grouped data to draw pie chart
7998dd2 
### What is this PR for?
Now, grouped pie charts only uses the data from the first group
With this fix-
* Add data from all groups to variable d3g, so all groups could be rendered
* Rewrite for loop with map and concat
* Refactor some variables to const and let

### What type of PR is it?
[Bug Fix]

### What is the Jira issue?
*  [ZEPPELIN-2237](https://issues.apache.org/jira/browse/ZEPPELIN-2237)

### How should this be tested?
* Create a built in pie chart visualization
* Select a column to group the data
* Should display the visualization based on all the available grouped data

### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No

Author: ess_ess <sravans2011@gmail.com>

This patch had conflicts when merged, resolved by
Committer: Lee moon soo <moon@apache.org>

Closes #2128 from sravan-s/ZEPPELIN-2237-grouped-piechart and squashes the following commits:

652c943 [ess_ess] Make use of all grouped data to draw pie chart

(cherry picked from commit a2cd4ae)
Signed-off-by: Lee moon soo <moon@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@sravan-s @Leemoonsoo