Skip to content

summarize_layout does not return final range values #2895

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

Closed
schloerke opened this issue Sep 17, 2018 · 9 comments
Closed

summarize_layout does not return final range values #2895

schloerke opened this issue Sep 17, 2018 · 9 comments

Comments

@schloerke
Copy link
Contributor

schloerke commented Sep 17, 2018

@clauswilke

Brushing points within shiny apps depends on the output of ggplot2::summarise_layout(p) CRAN ggplot2 3.0.0 works as expected.

With changes from #2832 , the range values of CoordTrans where changed to map back to their original values. While I agree with the intent, ggplot2::summarise_layout uses the range values to calculate the returned domain value.

library(ggplot2); 
dat <- data.frame(xvar = c(10^-1, 10^3), yvar = c(2^-2, 2^4))
p <- ggplot(dat, aes(xvar, yvar)) + geom_point() +
  scale_x_log10(expand = c(0 ,0)) +
  scale_y_continuous(expand = c(0, 0)) +
  coord_trans(y = "log2")
p

Created on 2018-09-17 by the reprex package (v0.2.0).

With v3.0.0 the fully transformed scale values provide:

ggplot2::summarise_layout(ggplot2::ggplot_build(p))
#> # A tibble: 1 x 10
#>   panel   row   col vars     xmin  xmax  ymin  ymax xscale        yscale       
#>   <fct> <dbl> <dbl> <list>  <dbl> <dbl> <dbl> <dbl> <list>        <list>       
#> 1 1         1     1 <list …    -1     3    -2     4 <S3: ScaleCo… <S3: ScaleCo…

With current github version the fully transformed scale values provide:

# create `p` again
ggplot2::summarise_layout(ggplot2::ggplot_build(p))
#> # A tibble: 1 x 10
#>   panel   row   col vars     xmin  xmax  ymin  ymax xscale        yscale       
#>   <fct> <dbl> <dbl> <list>  <dbl> <dbl> <dbl> <dbl> <list>        <list>       
#> 1 1         1     1 <list …    -1     3  0.25    16 <S3: ScaleCo… <S3: ScaleCo…

The ymin and ymax are displaying the original values, not the fully transformed values.


Shiny demo... Try selecting the two points at x = 150. In the dev version, select the very bottom of the plot (around y = 2) are at x = 150 to brush the transformed data locations.

library(ggplot2)
library(shiny)

ui <- fluidPage(plotOutput("plot", brush = "brush"), tableOutput("table"))
server <- function(input, output, session) {
  output$plot <- renderPlot({
    ggplot(mtcars, aes(hp, wt)) + geom_point() +
      coord_trans(y = "log2")
  })
  output$table <- renderTable({
    brushedPoints(mtcars, input$brush)
  })
}
shinyApp(ui, server)

@wch @jcheng5

@clauswilke
Copy link
Member

clauswilke commented Sep 17, 2018

Thanks for the report. This is a tricky issue. I doubt we can fully resolve this in the near term, but let's see if we can make some progress.

First, I'm not sure ggplot2 3.0.0 works as expected. Please try the following example:

library(ggplot2)
library(shiny)

ui <- fluidPage(plotOutput("plot", brush = "brush"), tableOutput("table"))
server <- function(input, output, session) {
  output$plot <- renderPlot({
    ggplot(mtcars, aes(hp, wt)) + geom_point() +
      coord_polar()
  })
  output$table <- renderTable({
    brushedPoints(mtcars, input$brush)
  })
}
shinyApp(ui, server)

Second, the problem fundamentally is that nobody actually knows what coord$range() does. It is not documented and it is used and implemented inconsistently throughout ggplot2. From my analysis, I thought it was primarily used to backtransform ranges, but now from looking at summarise_layout() once more, I think that in that case it's actually used to convert panel params into ranges. For standard cartesian coordinate system those two things are the same, but for any other coordinate systems that is not necessarily the case.

Third, summarise_layout() is also undocumented and it is not clear what it is exactly meant to do.

A possible fix, which #2821 does not currently provide but could be expanded into, is to have both a function range() and a function backtransform_range(), where range() would behave mostly like the range() function in ggplot2 3.0.0 and backtransform_range() would behave mostly like the range() function in current master, and the summarise_layout() function then would call range() not backtransform_range(). (In #2821, it calls backtransform_range(), which is why I'm surprised that helps you in any way.)

@clauswilke clauswilke mentioned this issue Sep 17, 2018
25 tasks
@jcheng5
Copy link

jcheng5 commented Sep 17, 2018

@clauswilke FWIW, coord_polar has never worked with Shiny interactive (in the click, hover, brush sense) graphics with any version of ggplot2--we've considered it a known issue.

@clauswilke
Copy link
Member

@jcheng5 Ok. It doesn't seem to work with coord_sf() either (see example below). My point was that getting brushing to work in all combinations of coordinate systems and scale transformations is going to be difficult, and we may have to think more deeply about what is the correct way to support this function.

In any case, I think the fix I proposed above should work for now. I'll prepare a PR within the next few days, so we can try it.

library(sf)
nc <- sf::st_read(system.file("shape/nc.shp", package="sf"), quiet = TRUE)

library(ggplot2)
library(shiny)

ui <- fluidPage(plotOutput("plot", brush = "brush"), tableOutput("table"))
server <- function(input, output, session) {
  output$plot <- renderPlot({
    ggplot(nc) +
      stat_sf_coordinates() + coord_sf()
  })
  output$table <- renderTable({
    brushedPoints(mtcars, input$brush)
  })
}
shinyApp(ui, server)

@clauswilke
Copy link
Member

@jcheng5 One more question: Could you explain how brushing currently works? How do you go from the ranges of the layout summary to the original data?

@smouksassi
Copy link

Brushing has always had some issues refer to rstudio/shiny#1433 even without transformation the example show a problem with discrete scales.

@clauswilke
Copy link
Member

I have updated #2821 so that summarise_layout() mirrors as closely as possible the 3.0.0 behavior while all the fixes that #2832 introduced remain in place. Could you test this?

Here is the example from @schloerke:

library(ggplot2); 
dat <- data.frame(xvar = c(10^-1, 10^3), yvar = c(2^-2, 2^4))
p <- ggplot(dat, aes(xvar, yvar)) + geom_point() +
  scale_x_log10(expand = c(0 ,0)) +
  scale_y_continuous(expand = c(0, 0)) +
  coord_trans(y = "log2")
p

ggplot2::summarise_layout(ggplot2::ggplot_build(p))
#> # A tibble: 1 x 10
#>   panel   row   col vars        xmin  xmax  ymin  ymax xscale    yscale   
#>   <fct> <dbl> <dbl> <list>     <dbl> <dbl> <dbl> <dbl> <list>    <list>   
#> 1 1        1.    1. <list [0]>   -1.    3.   -2.    4. <S3: Sca… <S3: Sca…

Created on 2018-09-17 by the reprex package (v0.2.0).

clauswilke added a commit to wilkelab/ggplot2_archive that referenced this issue Sep 18, 2018
@schloerke schloerke changed the title summarize_layout does not returning final range values summarize_layout does not return final range values Sep 18, 2018
@schloerke
Copy link
Contributor Author

@clauswilke #2821 works for my small litmus tests. I see the range function added back to coord-trans, so I don't expect any more range errors.

Thank you for the documentation in summarize_layout and for (down the road) solidifying what range actually means!

@clauswilke
Copy link
Member

@hadley This issue is blocking the release of 3.0.1. I see only two feasible ways forward:

  1. Revert Fix back-transformation of ranges in coords, without API cleanup #2832 and defer to a later date.
  2. Merge Fix back-transformation of ranges in coords #2821. This PR was originally introducing a breaking API change, but I think I have diffused that problem now. As it stands, the current master is less backwards compatible than it would be with Fix back-transformation of ranges in coords #2821 merged.

My own preference is option 2.

@lock
Copy link

lock bot commented Mar 24, 2019

This old issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with reprex) and link to this issue. https://reprex.tidyverse.org/

@lock lock bot locked and limited conversation to collaborators Mar 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants