thomasp85 / patchwork

The Composer of ggplots
https://patchwork.data-imaginist.com
Other
2.45k stars 160 forks source link

Can't save patchwork object in a list column in data.table: a possible conflict between patchwork and data.table. #381

Closed person-c closed 1 week ago

person-c commented 2 weeks ago

see https://github.com/Rdatatable/data.table/issues/6395 and https://stackoverflow.com/questions/77085628/out-of-bound-error-when-trying-to-combine-plots-using-data-table

r$> mtDT <- setDT(copy(mtcars))

    mtDT[, .(plot = list(ggplot(data = .SD) + geom_point(aes(x=wt, y=mpg)))), by=.(carb,am)][,
    .(list(patchwork::wrap_plots(plot))), by=am]
Error in `X[[i]]`:
! Index out of bounds
Run `rlang::last_trace()` to see where the error occurred.

r$> rlang::last_trace(drop = FALSE)
<error/rlang_error>
Error in `X[[i]]`:
! Index out of bounds
---
Backtrace:
     x
  1. +-...[]
  2. \-data.table:::`[.data.table`(...)
  3.   \-data.table (local) runlock(ans)
  4.     \-base::lapply(x, runlock, current_depth = current_depth + 1L)
  5.       \-data.table (local) FUN(X[[i]], ...)
  6.         \-base::lapply(x, runlock, current_depth = current_depth + 1L)
  7.           \-data.table (local) FUN(X[[i]], ...)
  8.             \-base::lapply(x, runlock, current_depth = current_depth + 1L)
  9.               +-X[[i]]
 10.               \-patchwork:::`[[.patchwork`(X, i)
 11.                 \-cli::cli_abort("Index out of bounds")
 12.                   \-rlang::abort(...)
thomasp85 commented 2 weeks ago

My gut feeling is that this is because patchwork allows for indexing into the plots in the patchwork using [[ but keeps the length method of the parent ggplot class to avoid messing more than necessary with it. This however means that patchwork objects breaks the promise that length returns the maximum number of elements indexable by [[... let me see if there is a way around this

thomasp85 commented 1 week ago

hmm... that assessment was wrong since I actually report the right length based on indexing behaviour

thomasp85 commented 1 week ago

See my comment on the data.table issue. I don't think there is much I can do at this point

thomasp85 commented 1 week ago

I was mistaken. Providing an as.list() method for the patchwork object forced lapply() into doing the right thing