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

How can I completely remove the legend? #61

Closed
Geoffrey-Guest opened this issue Feb 16, 2018 · 26 comments
Closed

How can I completely remove the legend? #61

Geoffrey-Guest opened this issue Feb 16, 2018 · 26 comments
Assignees

Comments

@Geoffrey-Guest
Copy link

I'm looking for a way to remove the initialization of the legend. I've commented out all code that I thought was to do with the legend (in sunburst.js) and now the Legend toggle doesn't work. However, I don't want to see the Legend label and toggle. Any idea what I'm missing? Thanks a lot.

@timelyportfolio
Copy link
Owner

timelyportfolio commented Feb 16, 2018

The legend has been a problem for many users, so I propose adding an argument to disable - maybe something like legend=FALSE. I will try to get this implemented over the weekend and combine with b8a18d1 for version 2.0.0 release to submit next week to CRAN.

While it is possible currently to remove, it is difficult and hacky.

@Geoffrey-Guest
Copy link
Author

Thanks, that's highly appreciated! It's boggling my mind where in the code the label 'Legend' with the checkbox is actually created. I simply don't see it the sunburst.js file. But to have the option to remove it that would be great. The main reason I want to remove it is that it becomes too lengthy and therefore impractical.

@timelyportfolio
Copy link
Owner

@Geoffrey-Guest, the reason for the confusion is the custom html on the R side in lines. In general, I favor the more general htmlwidget practice of an empty div that then gets populated from JS, and my idea would be to move this direction with the proposed change.

@timelyportfolio
Copy link
Owner

timelyportfolio commented Feb 20, 2018

@Geoffrey-Guest, would you mind trying sunburst(..., legend = FALSE) after installing the newest with devtools::install_github("timelyportfolio/sunburstR")? Please let me know if you think piggybacking on what was the legend config argument is confusing.

I opted to keep the custom sunburst_html() as is to be less disruptive.

@Geoffrey-Guest
Copy link
Author

Geoffrey-Guest commented Feb 21, 2018

@timelyportfolio thanks for doing this. I tested it out and it looks to work quite well. One small thing is when the sunburst renders you can see the Legend label with checkbox but it disappears as soon as the figure is finished rendering, so not a big deal.

I'm not sure what you mean by "piggybacking on what was the legend config argument is confusing."

Another issue I'm facing involves the overlap of the breadcrump with the figure. It was especially apparent in a knitr html Rmd test run (see attached). Unfortunately I cannot attach html files.
screenshot 2018-02-21 14 17 15

For a reason I don't know, the sunburst doesn't render in the html knitr file if I specify the width/height in sunburst(...).

Anyways I played around with the "// Dimensions of sunburst" in "sunburst.js" to get dimensions that suited me best.

thanks again.

@Geoffrey-Guest
Copy link
Author

Geoffrey-Guest commented Feb 21, 2018

Actually, there might be a calculation error in your code now because the breakdown is no longer correct:

BEFORE:
screenshot 2018-02-21 16 49 01

AFTER:
screenshot 2018-02-21 16 49 18

I'll try my best to make sure it wasn't me. I did check the final table going into sunburst and the % for the hovered path as above did agree with the BEFORE image.

@timelyportfolio
Copy link
Owner

timelyportfolio commented Feb 21, 2018

@Geoffrey-Guest, thanks for testing...

legend
I did not notice the legend showing up and then disappearing, but it does not surprise me. I will fix this by changing sunburst_html and then adding the toggle with JavaScript.

breadcrumb
Not sure how best to handle the overlapping breadcrumb...so will have to think on this one. This might unfortunately have to wait for a while. I don't think an expanding chart window would be pleasant. Adding logic to fill around the sunburst seems like it would be difficult. Laying out with different size of sunburst might be the best option, but I remember layout was particularly problematic when I first built the widget. If we are ok with reducing the size of the sunburst chart, then I might be able to add a margin argument to allow more room for the breadcrumb.

calculation error
This most likely stems from #60 (comment) "fixed" with b8a18d1. Is your tree pre-summed and might the before be incorrect? Could you provide a small sample of your data, so that I could test?

@Geoffrey-Guest
Copy link
Author

No I didn't pre-sum it. Each value represents the contribution of its respective pathway.

You can use the csv I uploaded...

sequences = read.csv("sequences.csv",header = TRUE)

sb <- sequences %>%
arrange(desc(depth), path) %>%
sunburst(width="100%",height = "100%", legend = FALSE)

sb

sequences.zip

@timelyportfolio
Copy link
Owner

@Geoffrey-Guest, is the aggregation not a sum? Also I could not get the data to work in sunburst, so I could not test (sorry). Let's take something like. With your data what would be size?

df <- data.frame(
  index1 = c(rep("A",3),"B"),
  index2 = c(NA,"A.1","A.1",NA),
  index3 = c(NA, NA, "A.1.1", NA),
  size = c(5,5,5, 10),
  stringsAsFactors = FALSE
)

Here is what I tried.

library(sunburstR)
library(d3r)
library(dplyr)
library(tidyr)

sequences <- read.csv(
  "c:/users/kentr/downloads/sequences.csv",
  header = TRUE
)

sequences %>%
  arrange(desc(depth), path) %>%
  sunburst(width="100%",height = "100%", legend = FALSE)

df <- bind_cols(
  do.call(
    bind_rows,
    lapply(
      strsplit(as.character(sequences$path), split="-"),
      function(x) {
        vv <- unlist(x)
        names(vv) <- paste0("index", seq_along(vv))
        vv
      }
    )
  ),
  sequences[,-1]
)

df %>%
  d3r::d3_nest(value_cols = c("value", "depth")) %>%
  sunburst(
    valueField = "value",
    legend = FALSE,
    width="100%",height = "100%"
  )

@Geoffrey-Guest
Copy link
Author

Geoffrey-Guest commented Feb 23, 2018

If you aggregate the value column yes that would be the total.

The size in your df should be the same parameter as my value in sequences. The sum of all values is the total.

The code worked for me with the Cran version I had downloaded. I adapted my data to conform to one of your examples. I tried both your coding attempts and they both worked with the Cran version but not your github version. It looks like the issue in terms of it not rendering is the width and height arguments. For example, these all worked with your github version (though erroneous breakdown):

sunburst(sequences)
#or
sequences %>%
  arrange(desc(depth), path) %>%
  sunburst(legend = FALSE)

#or

df %>%
  d3r::d3_nest(value_cols = c("value", "depth")) %>%
  sunburst(
    valueField = "value",
    legend = FALSE
  )


I'm not sure what changed. How should I organize my data now?

@timelyportfolio
Copy link
Owner

@Geoffrey-Guest, thanks for the additional information. I most definitely don't want data to break with changes, so I'd like to use your example data as a test case to determine why the change does not work with your data. I'll spend some time over the weekend working through the problem.

If possible, could you provide session info for the working version? It sounds like current CRAN works, so this is not critical if it is too much hassle.

@Geoffrey-Guest
Copy link
Author

Okay thanks for looking into it. Here's the session info:

other attached packages:
[1] sunburstR_1.0.3

loaded via a namespace (and not attached):
[1] htmlwidgets_0.9 compiler_3.4.2 htmltools_0.3.6 Rcpp_0.12.14
[5] digest_0.6.12

@timelyportfolio
Copy link
Owner

timelyportfolio commented Feb 25, 2018

I installed CRAN version, and the example is not working. In console I see,

image

... so I went back farther to 0.6.4 and same error.

... so then I thought since small numbers, try options(digits = 20) and it worked.

@timelyportfolio
Copy link
Owner

timelyportfolio commented Feb 25, 2018

@Geoffrey-Guest, back to the original issue on whether before or after is correct, I used treemap to calculate sums, and this suggests after is the correct representation.

library(sunburstR)
library(d3r)
library(dplyr)
library(tidyr)


options(digits = 20)

#sequences <- read.csv(
#  "c:/users/kentr/downloads/sequences.csv",
#  header = TRUE
#)

df <- bind_cols(
  do.call(
    bind_rows,
    lapply(
      strsplit(as.character(sequences$path), split="-"),
      function(x) {
        vv <- unlist(x)
        names(vv) <- paste0("index", seq_along(vv))
        vv
      }
    )
  ),
  sequences[,-1]
)

sunburst(
  d3_nest(
    df, value_cols=c("value","depth")
  ),
  valueField="value"
)

library(treemap)
tm <- treemap(
  df,
  index = paste0("index", 1:14),
  vSize = "value",
  palette = blues9,
  draw = FALSE
)$tm

sunburst(
  d3_nest(tm[,c(1:15)], value_cols="vSize"),
  valueField = "vSize"
)

image

@Geoffrey-Guest
Copy link
Author

Geoffrey-Guest commented Feb 25, 2018

thanks for investigating @timelyportfolio . I don't think the after is correct. As an example take the contributing pathway "Other Pathways...." (last row in dataset and largest contributor in the after) and calculate the % contribution and it is equal to 10.2% (0.26259/2.574) which is equivalent to the 'BEFORE' whereas the 'AFTER' = 52.7%. So perhaps our definitions of what value should be are different or you changed the definition in your latest version. For me each value is the contribution that the pathway is responsible for and sum(value) is the total contribution across all pathways. Perhaps your latest release ignores the really small values...?

@timelyportfolio
Copy link
Owner

@Geoffrey-Guest, I figured it out. In your data, children do not sum to parent, and I don't think imposing this constraint would be good, so I have started #62. Getting closer to a solution I think...

@timelyportfolio
Copy link
Owner

@Geoffrey-Guest, if possible, will you please test with devtools::install_github("timelyportfolio/sunburstR")? Thanks for all your help with this.

@timelyportfolio
Copy link
Owner

timelyportfolio commented Feb 25, 2018

@Geoffrey-Guest, in addition, you might want to try the new d2b version.

#devtools::install_github("timelyportfolio/sunburstR")
library(sunburstR)
library(d3r)
library(dplyr)
library(tidyr)

#sequences <- read.csv(
#  "c:/users/kentr/downloads/sequences.csv",
#  header = TRUE
#)

df <- bind_cols(
  do.call(
    bind_rows,
    lapply(
      strsplit(as.character(sequences$path), split="-"),
      function(x) {
        vv <- unlist(x)
        names(vv) <- paste0("index", seq_along(vv))
        vv
      }
    )
  ),
  sequences[,-1]
)

sund2b(
  d3_nest(
    df, value_cols=c("value","depth")
  ),
  valueField="value"
)

image

@Geoffrey-Guest
Copy link
Author

oh wow amazing you got that new version up and running! THanks for all your efforts in making it work for me!
I'll test it tonight or tomorrow.

@Geoffrey-Guest
Copy link
Author

Geoffrey-Guest commented Feb 26, 2018

Both are working but with a few limitations/issues:
for sunburst(): if the legend isn't selected (ie legend = FALSE) it would be nice if the graph and the breadcrumbs took the entire width.

for sund2b(): the absolute values are being rounded to the nearest value with 0 decimal places so the report values aren't useful for me right now. It would be good to always keep absolute and % values with 3 significant digits. Also, for the main contributing pathway, for some reason two extra layers are being tacked on but they shouldn't be there (i.e. diesel burned in building machine). Also, it would be nicer if the breadcrumbs were wider so I would be able to see a long trail more easily on the screen.

ONe other thing I was thinking about... I like sunburst() because it has functionality to do something on-click and I set it up in a shiny app and when a user clicks a segment, greater details are presented in a table/figure. WIll this click-segment functionality be lost with the sund2b visual because a click results in a zoomable or is it possible to have both?

Other than that it looks really good, very promising!

@timelyportfolio
Copy link
Owner

I just checked, and when the legend is absent (legend = FALSE) the breadcrumbs should take up the entire width. The breadcrumbs will wrap though if they do not fit in the chart window, so some space will be left depending on the breadcrumb width.

image

The author Kevin Warne of d2b deserves the credit for the great functionality and highly useful sunburst. I have asked d2bjs/d2b#14 if he might be interested in considering a format argument.

I plan to add Shiny to d2b, so will let you know when I get that implemented.

@Geoffrey-Guest
Copy link
Author

Geoffrey-Guest commented Feb 26, 2018

Thanks, sorry you're right, with sunburst() I do get the entire width with no legend. I guess because the viewer window was small at first and then I maximized the window and the width didn't update to the new window size. At this point for my current purposes I tend to like your original sunburst() over the d2b sunburst. I think it's more intuitive and the zooming might get confusing to some users.

There was indeed a rendering issue as I mentioned earlier. In the sequences df, you can see the pathway is captured correctly and even the values are right but the ??segment arc length?? is too big (should only be 0.4%) and it's a lot larger. It's row 7 in the df. If you compare the two sunbursts side-by-side it looks like there might be other issues too. Right now I trust the rendered figure from sunburst() more than sund2b() right now. Do you get the same error?

@timelyportfolio
Copy link
Owner

@Geoffrey-Guest, we lose some control over the sum/aggregate with d2b, so you might be right that d2b suffers some of the problems we had earlier. I now see the 12720/diesel issue. I'll look into it.

@Geoffrey-Guest
Copy link
Author

thanks a lot @timelyportfolio
Also, I was curious, do you think the treeSankey diagram could easily be applied to my dataset and does it work with Shiny? If so I'd love to test it out.

@timelyportfolio
Copy link
Owner

@Geoffrey-Guest, since your data is in a tree structure, yes Sankey should work, but given the level of depth, it might not render well in constrained space. I'll try to add an example.

@timelyportfolio
Copy link
Owner

going to close. please feel free to reopen. I'll reference https://github.com/fbreitwieser/hiervis for sankey tree.

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

2 participants