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

Pivottable is now able to render multi Donut charts using Plotly renderer. #990

Closed

Conversation

ebellumat
Copy link
Contributor

@ebellumat ebellumat commented Aug 16, 2018

Pivottable is now able to render multi Donut charts using Plotly renderer.

Related issues: #66 #256 #457 #893 #876
People interested on this: @kevin5657 @rjst , @worstenbrood, @rajuchapagain, @tgabrielle @kumarabrol @ambarishbh

image
image
image

@ebellumat ebellumat changed the title Feature/plotly pie Pivottable is now abble to render multi Donnut charts using Plotly renderer. Aug 16, 2018
@ebellumat ebellumat changed the title Pivottable is now abble to render multi Donnut charts using Plotly renderer. Pivottable is now able to render multi Donut charts using Plotly renderer. Aug 16, 2018
@nicolaskruchten
Copy link
Owner

This is very neat, thank you! Unfortunately I'm just headed out on vacation so I will likely not have time to review it for another 10 days or so but I should be able to merge it thereafter.

My main comment right now is that we need some way of identifying the donuts, some kind of title or something. Either inside the middle or above.

@ebellumat
Copy link
Contributor Author

ebellumat commented Aug 17, 2018

Thanks for your time, don't hurry to review the code, probably i will have to do some adjustments after your review.

Anyway, i created a issue on Plotly JS repository, number #2915 asking to automatic show a annotation with a name of a pie chart, @alexcjohnson did the analysis and generated the issue #2916 "Grid-referenced components"

You can use layout.grid to position subplots, it would be nice if you could position annotations, shapes, images, and other items like colorbars and sliders, with reference to grid cells as well. Perhaps something like:

Actually, only on hover the name of a pie is displayed, i will make a way to display the name of the pie's, as suggested, but, with #2915 solved or #2916 solved, it would be much easier, haha.

@ebellumat
Copy link
Contributor Author

#2925 Closed in Plotly!!

@nicolaskruchten
Copy link
Owner

nicolaskruchten commented Sep 29, 2018 via email

@nicolaskruchten
Copy link
Owner

First of all, thanks again for the impetus here to finally get a multi-pie implementation done. That said, here are a few notes here on why I think I'll make a slightly different implementation than the one in this PR:

  1. I will use the scalegroup feature of plotly.js pie charts such that pies are scaled with respect to each other, so that they are a clear analogy to stacked bar charts
  2. As a result I will not want to use the hole parameter to turn them into donuts, as in that case they areas are no longer adequately comparable
  3. Similarly to the way the other plotly_renderer renderers work, I'll disable the text on the actual pie slices: the text isn't displayed on bars in bar charts right now so it shouldn't be displayed on pie charts either.
  4. I would like some slightly smarter handling of large numbers of pies: 10 pies should render in rows of 4/4/2, for example, and 20 pies should be 5/5/5/5 and so on. I'm not sure what the exact algorithm should be but I'll play with it a bit.
  5. This is a minor thing, but important to me: the CoffeeScript style in this PR is quite different from the style in the rest of the file, which would likely cause maintenance problems later.

@ebellumat
Copy link
Contributor Author

I would like to take the occasion to say thank you and confess. First of all thanks for considering the implementation of this feature in the main project, this is the first project I do a pull request. I've used this lib since 2016, =)

@ghost
Copy link

ghost commented Oct 13, 2018

Please consider including the ability to (optionally) make these donut charts where you can write text in the center of them. Although they are inferior to pie charts in terms of proper data visualization, business mangers (the people most of us make our reports for) really like them and even specifically ask for them :)

Thank you! This library rocks, I use rpivottable heavily!

@nicolaskruchten
Copy link
Owner

Thanks for your input @Blake-Eryx ! Unfortunately I don't think I'll be able to do this in the built-in renderers in the short run, partly for the datavis related reasons laid out above, and also because laying out donut chart titles in the middle of the donut is not a built-in feature of plotly.js. In general this would be a tough problem to solve because the 'hole' in a donut chart is often quite small compared the amount of text a label can contain, and automatically wrapping text in SVG charts is a bit challenge.

@alexcjohnson
Copy link

laying out donut chart titles in the middle of the donut is not a built-in feature of plotly.js

In fact, this will be included in plotly.js v1.42, due out in ~2 weeks plotly/plotly.js#2987 🎉

@nicolaskruchten
Copy link
Owner

nicolaskruchten commented Oct 13, 2018 via email

@alexcjohnson
Copy link

Correct, a donut is just a pie with a hole as far as plotlyjs is concerned. And titles, per that PR, work either way - by default they’re above pies and inside donuts, but that too is templatable.

@nicolaskruchten nicolaskruchten mentioned this pull request Nov 4, 2018
@nicolaskruchten
Copy link
Owner

Thanks again for the prompting with this PR! I've got #1041 in development and it will do everything this PR does except for the hole to make pies into donuts.

@deepsweech
Copy link

Is this going to be available for react-pivottable.js?

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.

4 participants