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

convert to d3v4 #36

Closed
timelyportfolio opened this issue Jun 7, 2017 · 15 comments
Closed

convert to d3v4 #36

timelyportfolio opened this issue Jun 7, 2017 · 15 comments

Comments

@timelyportfolio
Copy link
Owner

timelyportfolio commented Jun 7, 2017

This is part of a grand refactor. I plan to use Github Projects to track and manage the conversion, so see https://github.com/timelyportfolio/sunburstR/projects/1 to monitor, cheer, or help.

@timelyportfolio timelyportfolio changed the title convert to d3v4 and refactor JS code in process convert to d3v4 Jun 8, 2017
This was referenced Jun 8, 2017
@timelyportfolio
Copy link
Owner Author

Will track with d3v4 branch https://github.com/timelyportfolio/sunburstR/tree/d3v4 so please direct any pulls there.

@cjyetman
Copy link
Collaborator

I have a functioning version of sunburstR converted to D3v4 here: https://github.com/cjyetman/sunburstR/tree/d3v4

It can be installed and tested with:

devtools::install_github('cjyetman/sunburstR', ref = 'd3v4')

It would be great if a few people could test it out and give some feedback.

I'll make a PR as soon as @timelyportfolio gives the thumbs up.

@timelyportfolio
Copy link
Owner Author

pr away !! Thanks!

@timelyportfolio
Copy link
Owner Author

Thanks!!!!!! All I found were some examples that still used d3v3 api in the appended JS. After a little more testing and maybe some feedback (very hard to get), I will close this and move on to the other items in the project https://github.com/timelyportfolio/sunburstR/projects/1. Thanks.

@cjyetman
Copy link
Collaborator

pinging @msusol here because he was eager for a D3v4 version in #22... you can install the dev version with this and give feedback...

devtools::install_github('timelyportfolio/sunburstR', ref = 'd3v4')

@msusol
Copy link

msusol commented Jun 10, 2017

Something seems odd about the display of the slices selected?

I would expect the "path" to be [ 4360 > 22 > 16.8% where 4360 is one color, 22 is another color. It seems the text is a concat.

sunburstr_d3v4_test1

@timelyportfolio
Copy link
Owner Author

@msusol that is strange. Which sequences.csv are you using? sunburstR still requires lots of testing, so I would love to figure this out.

@msusol
Copy link

msusol commented Jun 10, 2017

sunburst.csv.zip

@cjyetman
Copy link
Collaborator

I think the levels have to be separated by a '-', no? Which is unfortunate in your case because you have some negative numbers in your path names.

@msusol
Copy link

msusol commented Jun 10, 2017

forgot how that file was created, but there are no negative numbers, replacing ">" with "-" fixed that issue.

Now what's missing, is the total number of sequences that take this path. so for 16.8% for 4360 > 22 sequence, would like to see under the path chosen visual, the total number of sequences under 4360, then under "22" 16.8% of the total.

4360 > 22
238 (100%) > 40 (16.8%)

I have seen a sunburst done where this is done vertically where your legend is displayed. so there is a gap between the colored blocks where the numbers are drawn.

@timelyportfolio
Copy link
Owner Author

The JavaScripy hierarchy can fail quite easily. When I get some time I will try to see if this is a hierarchy construction issue or a vis layout issue. Thanks for sharing the file.

@msusol
Copy link

msusol commented Jun 10, 2017

here is updated csv
sunburst.csv2.zip

@cjyetman
Copy link
Collaborator

@msusol I'm not sure I understand what you're asking? Is this something not working the way it should (a bug), or is this a new feature you would like to see added? If it's a new feature you'd like to see added, it would be better if you opened a new issue for that.

Have you tried the count = TRUE argument? It adds the count and total in the explanation in the middle of the sunburst.

sunburst(sequences, count = T)

@timelyportfolio
Copy link
Owner Author

timelyportfolio commented Jun 11, 2017

Ok, finally got a chance to look at this and read more carefully. @msusol, it seems like you would like to know how to pursue more advanced customization of the ultimate visual. Is this correct? Are you talking about something like this example from d2b?

image

If so, a result like the d2b example is currently out of the scope for the existing sunburst function. However, I had already mentioned to @cjyetman that I am considering adding another widget for creating a d2b sunburst within sunburstR. I will need some time to properly integrate though, and I think completing the conversion is higher priority and also a prerequisite before pursuing this. As confirmation of my interest, see d2bjs/d2b#1.

If this is the case, I feel like the best thing to do is open another issue to track progress toward this goal. Would love to hear your thoughts?

@timelyportfolio
Copy link
Owner Author

@cjyetman, seems this is complete, so moving on to #37 and #40. Thanks!!!!!

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

No branches or pull requests

3 participants