From 9ceefc844a2678bc312f0293b39726758ff096a5 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Tue, 13 Aug 2019 11:24:27 +0800 Subject: [PATCH 1/3] Closes #3633 -- restore reshape2 redirection (with warning) --- NEWS.md | 2 +- R/fcast.R | 18 +++++++++++++++--- R/fmelt.R | 18 +++++++++++++++--- 3 files changed, 31 insertions(+), 7 deletions(-) diff --git a/NEWS.md b/NEWS.md index f6a44b54d6..cc2630b0f5 100644 --- a/NEWS.md +++ b/NEWS.md @@ -244,7 +244,7 @@ 7. Added a note to `?frank` clarifying that ranking is being done according to C sorting (i.e., like `forder`), [#2328](https://github.com/Rdatatable/data.table/issues/2328). Thanks to @cguill95 for the request. -8. 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](https://github.com/Rdatatable/data.table/issues/3549). We thank the `reshape2` authors for their original inspiration for these functions. +8. 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 will begin to formally complete the split from `reshape2` by removing some last vestiges. In particular we now warn when redirecting to `reshape2` methods and will later error before ultimately completing the split; see [#3549](https://github.com/Rdatatable/data.table/issues/3549) and [#3633](https://github.com/Rdatatable/data.table/issues/3633). We thank the `reshape2` authors for their original inspiration for these functions and @ProfFancyPants for noticing regression issues in an earlier dev version. 9. `DT[col]` where `col` is a column containing row numbers of itself to select, now suggests the correct syntax (`DT[(col)]` or `DT[DT$col]`), [#697](https://github.com/Rdatatable/data.table/issues/697). This expands the message introduced in [#1884](https://github.com/Rdatatable/data.table/issues/1884) for the case where `col` is type `logical` and `DT[col==TRUE]` is suggested. diff --git a/R/fcast.R b/R/fcast.R index 976641d488..1981aa1f43 100644 --- a/R/fcast.R +++ b/R/fcast.R @@ -8,9 +8,21 @@ guess = function(x) { return(var) } -dcast = function(data, formula, fun.aggregate = NULL, ..., margins = NULL, - subset = NULL, fill = NULL, value.var = guess(data)) { - UseMethod("dcast", data) +dcast <- function( + data, formula, fun.aggregate = NULL, ..., margins = NULL, + subset = NULL, fill = NULL, value.var = guess(data) +) { + if (is.data.table(data)) UseMethod("dcast", data) + # nocov start + else { + # reshape2::dcast is not generic so we have to call it explicitly. See comments at the top of fmelt.R too. + ns = tryCatch(getNamespace("reshape2"), error=function(e) + stop("The dcast 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 dcast instead.")) + warning("The dcast generic in data.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$dcast(data, formula, fun.aggregate = fun.aggregate, ..., margins = margins, + subset = subset, fill = fill, value.var = value.var) + } + # nocov end } check_formula = function(formula, varnames, valnames) { diff --git a/R/fmelt.R b/R/fmelt.R index 2dabba3844..4df50c6692 100644 --- a/R/fmelt.R +++ b/R/fmelt.R @@ -1,9 +1,21 @@ # reshape2 dependency was originally abandoned because (1) we wanted to be in control # of the R version dependency and (2) reshape2::dcast is not generic. -# Anyway, reshape2 package is deprecated since December 2017. +# reshape2 package is deprecated since December 2017, so we'll deprecate our +# redirection as well -melt = function(data, ..., na.rm = FALSE, value.name = "value") { - UseMethod("melt", data) +melt <- function(data, ..., na.rm = FALSE, value.name = "value") { + if (is.data.table(data)) { + 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 + } } patterns = function(..., cols=character(0L)) { From 9db5bb1df6d42dd3bc7a26e6805acb91c146a4b3 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Wed, 14 Aug 2019 14:39:56 +0800 Subject: [PATCH 2/3] jans comments to improve usefulness of warning/error --- R/fcast.R | 6 +++--- R/fmelt.R | 9 +++++---- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/R/fcast.R b/R/fcast.R index 1981aa1f43..004cd1f9a3 100644 --- a/R/fcast.R +++ b/R/fcast.R @@ -15,10 +15,10 @@ dcast <- function( if (is.data.table(data)) UseMethod("dcast", data) # nocov start else { - # reshape2::dcast is not generic so we have to call it explicitly. See comments at the top of fmelt.R too. + data_name = deparse(substitute(data)) ns = tryCatch(getNamespace("reshape2"), error=function(e) - stop("The dcast 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 dcast instead.")) - warning("The dcast generic in data.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.") + stop("The dcast generic in data.table has been passed a ",class(data)[1L],", but data.table::dcast currently only has a method for data.tables. Please confirm your input is a data.table, with setDT(", data_name, ") or as.data.table(", data_name, "). If you intend to use a reshape2::dcast, try installing that package first, but do note that reshape2 is deprecated and you should be migrating your code away from using it.")) + warning("The dcast generic in data.table has been passed a ", class(data)[1L], " and will attempt to redirect to the reshape2::dcast; please note that reshape2 is deprecated, and this redirection is now deprecated as well. Please do this redirection yourself like reshape2::dcast(", data_name, "). In the next version, this warning will become an error.") ns$dcast(data, formula, fun.aggregate = fun.aggregate, ..., margins = margins, subset = subset, fill = fill, value.var = value.var) } diff --git a/R/fmelt.R b/R/fmelt.R index 4df50c6692..32e023f642 100644 --- a/R/fmelt.R +++ b/R/fmelt.R @@ -8,14 +8,15 @@ melt <- function(data, ..., na.rm = FALSE, value.name = "value") { 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 + # nocov start } else { - # nocov start + data_name = deparse(substitute(data)) 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.") + stop("The melt generic in data.table has been passed a ",class(data)[1L],", but data.table::melt currently only has a method for data.tables. Please confirm your input is a data.table, with setDT(", data_name, ") or as.data.table(", data_name, "). If you intend to use a method from reshape2, try installing that package first, but do note that reshape2 is deprecated and you should be migrating your code away from using it.")) + warning("The melt generic in data.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. To continue using melt methods from reshape2 while both libraries are attached, e.g. melt.list, you can prepend the namespace like reshape2::melt(", data_name, "). In the next version, this warning will become an error.") ns$melt(data, ..., na.rm=na.rm, value.name=value.name) - # nocov end } + # nocov end } patterns = function(..., cols=character(0L)) { From 60282afafe1da13e9f73a524455fb5269fab432c Mon Sep 17 00:00:00 2001 From: mattdowle Date: Mon, 26 Aug 2019 14:42:12 -0700 Subject: [PATCH 3/3] tweak to news item --- NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index cc2630b0f5..c8c2dd5935 100644 --- a/NEWS.md +++ b/NEWS.md @@ -244,7 +244,7 @@ 7. Added a note to `?frank` clarifying that ranking is being done according to C sorting (i.e., like `forder`), [#2328](https://github.com/Rdatatable/data.table/issues/2328). Thanks to @cguill95 for the request. -8. 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 will begin to formally complete the split from `reshape2` by removing some last vestiges. In particular we now warn when redirecting to `reshape2` methods and will later error before ultimately completing the split; see [#3549](https://github.com/Rdatatable/data.table/issues/3549) and [#3633](https://github.com/Rdatatable/data.table/issues/3633). We thank the `reshape2` authors for their original inspiration for these functions and @ProfFancyPants for noticing regression issues in an earlier dev version. +8. 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 will begin to formally complete the split from `reshape2` by removing some last vestiges. In particular we now warn when redirecting to `reshape2` methods and will later error before ultimately completing the split; see [#3549](https://github.com/Rdatatable/data.table/issues/3549) and [#3633](https://github.com/Rdatatable/data.table/issues/3633). We thank the `reshape2` authors for their original inspiration for these functions, and @ProfFancyPants for testing and reporting regressions in dev which have been fixed before release. 9. `DT[col]` where `col` is a column containing row numbers of itself to select, now suggests the correct syntax (`DT[(col)]` or `DT[DT$col]`), [#697](https://github.com/Rdatatable/data.table/issues/697). This expands the message introduced in [#1884](https://github.com/Rdatatable/data.table/issues/1884) for the case where `col` is type `logical` and `DT[col==TRUE]` is suggested.