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

Handling how data.table::melt is not passing to reshape2::melt.list method anymore. #3633

Closed
ProfFancyPants opened this issue Jun 6, 2019 · 8 comments · Fixed by #3763
Closed
Assignees
Milestone

Comments

@ProfFancyPants
Copy link

ProfFancyPants commented Jun 6, 2019

So a lot of my old code is breaking using the dev version of data.table (1.12.3). This isn't critical but I wanted to bring it up here before I start making changes willynilly.

By continuing to file this new issue / feature request, I confirm I have :

  1. searched the news.

This is what the news says about the issue:

NEWS.0
L605
4. data.table redefines melt generic and suggests reshape2 instead of import. As a result we don't have to load reshape2 package to use melt.data.table anymore. The reason for this change is that data.table requires R >=2.14, whereas reshape2 R v3.0.0+. Reshape2's melt methods can be used without any issues by loading the package normally.
L609
6. Because reshape2 requires R >3.0.0, and data.table works with R >= 2.14.1, we can not import reshape2 anymore. Therefore we define a melt generic and melt.data.table method for data.tables and redirect to reshape2's melt for other objects. This is to ensure that existing code works fine.

Minimal reproducible example

SCRIPT_LIST <- list(
  W3_ENGLISH = list(
    ## _W3_ENGLISH ####
    LANGUAGE = "English",
    NCOL_TXT = 1428L,
    NCHAR_LINE1 = 8366L,
    FILE_NAME_RE = "^[Ww][Jj][^Ee]",
    D_MACRO = "text"
  ),
  W3_SPANISH = list(
    ## _W3_SPANISH ####
    LANGUAGE = "Spanish",
    NCOL_TXT = 228L,
    NCHAR_LINE1 = 1363L,
    FILE_NAME_RE = "^[Ww][Mm][^Ee]",
    D_MACRO = "text"
  ),
  W4_ECAD = list(
    ## _W4_ECAD ####
    LANGUAGE = "English or Spanish",
    NCOL_TXT = 188L,
    NCHAR_LINE1 = 1780L,
    FILE_NAME_RE = "^[Ww][Jj][Ee][Cc][Aa][Dd]",
    D_MACRO = "text"
  )
)
list_headers <- c(A = "ASSESS", P = "PARAM", V = "VALUE")

lapply( SCRIPT_LIST, setattr, name = "varname", value = list_headers['P'])
setattr(SCRIPT_LIST,          name = "varname", value = list_headers['A'])

## Previously, data.table knew to call reshape2::melt for it's list method so
## this worked.
melt(SCRIPT_LIST, value.name = list_headers['V'])

## Now reshape2::melt has to be called explictly. 
as.data.table(reshape2::melt(SCRIPT_LIST, value.name = list_headers['V']))

Error in UseMethod("melt", data) :
no applicable method for 'melt' applied to an object of class "list"
Calls: melt

Was data.table going to provide a list method of some kind? How does one best provide a method for data.table to see when it calls melt from data.table's namespace?

melt <- function (data, ..., na.rm = FALSE, value.name = "value") 
{
    UseMethod("melt", data)
}

# Output of sessionInfo()

sessionInfo()
R version 3.6.0 (2019-04-26)
Platform: x86_64-w64-mingw32/x64 (64-bit)
Running under: Windows 10 x64 (build 17134)

Matrix products: default

locale:
[1] LC_COLLATE=English_United States.1252 LC_CTYPE=English_United States.1252 LC_MONETARY=English_United States.1252 LC_NUMERIC=C LC_TIME=English_United States.1252

attached base packages:
[1] stats graphics grDevices utils datasets methods base

other attached packages:
[1] TBmisc_1.1.00 data.table_1.12.3 magrittr_1.5 pryr_0.1.4 stringr_1.4.0.9000

loaded via a namespace (and not attached):
[1] pkgload_1.0.2 splines_3.6.0 Formula_1.2-3 assertthat_0.2.1 latticeExtra_0.6-28 cellranger_1.1.0 remotes_2.0.4 sessioninfo_1.1.1 pillar_1.4.0
[10] backports_1.1.4 lattice_0.20-38 glue_1.3.1 digest_0.6.19 RColorBrewer_1.1-2 checkmate_1.9.3 colorspace_1.4-1 htmltools_0.3.6 Matrix_1.2-17
[19] plyr_1.8.4 pkgconfig_2.0.2 devtools_2.0.2 purrr_0.3.2 scales_1.0.0 webshot_0.5.1 processx_3.3.1 stringdist_0.9.5.1 openxlsx_4.1.1
[28] htmlTable_1.13.1 tibble_2.1.1 ggplot2_3.1.1 usethis_1.5.0 withr_2.1.2 nnet_7.3-12 lazyeval_0.2.2 cli_1.1.0 survival_2.44-1.1
[37] crayon_1.3.4 memoise_1.1.0 evaluate_0.13 ps_1.3.0 beepr_1.3 fs_1.3.1 xml2_1.2.0 foreign_0.8-71 pkgbuild_1.0.3
[46] tools_3.6.0 prettyunits_1.0.2 munsell_0.5.0 cluster_2.0.8 zip_2.0.2 callr_3.2.0 packrat_0.5.0 compiler_3.6.0 tinytex_0.13.4
[55] rlang_0.3.4 grid_3.6.0 rstudioapi_0.10 htmlwidgets_1.3 base64enc_0.1-3 rmarkdown_1.13.1 testthat_2.1.1 gtable_0.3.0 codetools_0.2-16
[64] inline_0.3.15 curl_3.3 reshape2_1.4.3 R6_2.4.0 gridExtra_2.3 knitr_1.23.2 dplyr_0.8.1 Hmisc_4.2-0 rprojroot_1.3-2
[73] desc_1.2.0 stringi_1.4.4 parallel_3.6.0 Rcpp_1.0.1 rpart_4.1-15 acepack_1.4.1 audio_0.1-6 tidyselect_0.2.5 xfun_0.7

@franknarf1
Copy link
Contributor

I think you missed this NEWS item from a couple weeks ago, when passing to reshape2 was removed, if I understand correctly https://github.com/Rdatatable/data.table/pull/3577/files#diff-8312ad0561ef661716b48d09478362f3

  1. Historically, dcast and melt were built as enhancements to reshape2's own dcast/melt. We removed dependency on reshape2 in v1.9.6 but maintained some backward compatibility. As that package has been deprecated since December 2017, we have now formally completed the split from reshape2 by removing some last vestiges, #3549. We thank the reshape2 authors for their original inspiration for these functions.

I use melt.list, melt.matrix and perhaps a few others with only data.table loaded. Since it is a breaking change for these use cases, maybe the NEWS should be more clear about the consequences, eg by starting the NEWS item with something like

melt will no longer redirect to methods found in the reshape2 package when the user has it installed but not loaded.

before giving the historical context/rationale/comments.

@MichaelChirico
Copy link
Member

Thanks guys. I wasn't aware these features are missing when we got rid of reshape2. I don't think we'll restore the redirect as reshape2 is deprecated. Will start with fixing the NEWS item to be more clear; ultimately, it may be best if we re-implement that functionality ourselves.

@MichaelChirico MichaelChirico self-assigned this Jun 7, 2019
@MichaelChirico MichaelChirico added this to the 1.12.4 milestone Jun 7, 2019
@jangorecki
Copy link
Member

jangorecki commented Jun 8, 2019

Maybe we could revert that change and add warning for now, so those who are not reading NEWS will be informed on coming breaking change.

@MichaelChirico
Copy link
Member

ok. still have a goal to get rid of it for 1.12.4

@odelmarcelle
Copy link
Contributor

odelmarcelle commented Dec 30, 2020

Hello,

I wanted to implement a melt method for an S3 object in a future package. Sadly, the behaviour of the data.table::melt generic only call UseMethod if the object is a data.table.

Would it be possible to alter this behaviour, calling UseMethod for all objects excepts for classes that need to be redirected to reshape2?

@MichaelChirico
Copy link
Member

@odelmarcelle I'm not sure this is possible without a Suggested dependency on reshape2

@odelmarcelle
Copy link
Contributor

Pardon my limited knowledge of the subject, but wouldn't it be possible to at least search among registered S3 methods before redirecting to reshape2? For example, using methods(melt)

I created a small reproducible example of this. First, an illustration of the issue I'm facing:

library(data.table)

# Creating a S3 object and it's melt method
S3Object = list(dt1 = data.table(id = 1:2, a = 3L), dt2 = data.table(id = 1:2, b = 4:5))
class(S3Object) = "S3Class"
new_melt = function(data, ..., na.rm = FALSE, value.name = "value") {
  melt(Reduce(merge, data), id.vars = "id")
}
.S3method("melt", "S3Class", new_melt)

melt(S3Object) ## Error because it redirects to reshape2

Perhaps I'm missing something important here, but redefining the condition on UseMethod seemed to work:

# Redefining generic
melt <- function(data, ..., na.rm = FALSE, value.name = "value") {
  if (inherits(data, sub("melt\\.", "", as.character(methods(melt))))) {
    UseMethod("melt", data)
    # if data is not data.table and reshape2 is installed, this won't dispatch to reshape2's method;
    # CRAN package edarf and others fail without the else branch
  } else {
    # nocov start
    ns = tryCatch(getNamespace("reshape2"), error=function(e)
      stop("The melt generic in data.table has been passed a ",class(data)[1L]," (not a data.table) but the reshape2 package is not installed to process this type. Please either install reshape2 and try again, or pass a data.table to melt instead."))
    warning("The dcast generic in melt.table has been passed a ", class(data)[1L], " and will attempt to redirect to the relevant reshape2 method; please note that reshape2 is deprecated, and this redirection is now deprecated as well.")
    ns$melt(data, ..., na.rm=na.rm, value.name=value.name)
    # nocov end
  }
}

# Re-registering methods
.S3method("melt", "data.table", getS3method("melt", "data.table"))
.S3method("melt", "S3Class", new_melt)

melt(S3Object) ## Desired outcome

# Still works with other classes
melt(data.table(a = 1, b = 2))
melt(data.frame(a = 1, b = 2))
melt(list(a = 1, b = 2))
melt(matrix(1:4, 2, 2))
melt(array(1:8, dim = rep(2,3)))
melt(table(rep(1:4, each = 2)))

@MichaelChirico
Copy link
Member

That sounds promising... would you be willing to write it into a PR?

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 a pull request may close this issue.

5 participants