Allow passing data.frame directly to dcast(), melt()

[manmita]

Closes #7614

I added tests for data.frame to check if dcast and melt work directly but got the following error

Running test id 2365.1         Test 2365.1 produced 1 errors but expected 0
Expected: 
Observed: no applicable method for 'melt' applied to an object of class "data.frame"
Running test id 2365.2         Test 2365.2 produced 1 errors but expected 0
Expected: 
Observed: no applicable method for 'dcast' applied to an object of class "data.frame"

Hence redirected data.frame to dcast.data.table directly similarly for melt

[github-actions]

• HEAD=fix/dcast_melt_for_data.frame stopped early for DT[,.SD] improved in #4501
  

Generated via commit c3650ea

Download link for the artifact containing the test results: ↓ atime-results.zip

Task	Duration
R setup and installing dependencies	2 minutes and 59 seconds
Installing different package versions	21 seconds
Running and plotting the test cases	4 minutes and 3 seconds

[MichaelChirico]

Just remove the error in dcast.data.table. Don't coerce.

[manmita]

got it

[MichaelChirico]

I think this constitutes a breaking change for packages/scripts using both {data.table} and {reshape2} (e.g. {WebAnalytics} or {BTSPAS}), which might result in the wrong S3 method being invoked.

Please investigate the interaction of your update with {reshape2}, e.g. the load order.

I think the solution is not to have a data.frame method, but to fake one in the dcast() generic like:

dcast = function(x, ...) {
  if (!is.data.table(x) && is.data.frame(x)) return(dcast.data.table(x, ...))
  UseMethod("dcast")
}

[manmita]

have redirected data.frame to the dcast.data.table
currently fixing a few errors

Running test id 2365.2         Test 2365.2 produced 1 errors but expected 0
Expected: 
Observed: argument "formula" is missing, with no default
Test 2365.2 produced 1 messages but expected 0
Expected: 
Observed: Using 'v' as value column. Use 'value.var' to override

Will update the PR once this is fixed

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.02%. Comparing base (b537ec4) to head (c3650ea).
⚠️ Report is 6 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #7634   +/-   ##
=======================================
  Coverage   99.02%   99.02%           
=======================================
  Files          87       87           
  Lines       16890    16896    +6     
=======================================
+ Hits        16726    16732    +6     
  Misses        164      164           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[manmita]

Hii @MichaelChirico without coercing it is failing the following test

df_dcast = data.frame(a = c("x", "y"), b = 1:2, v = 3:4)
dt_dcast = data.table(a = c("x", "y"), b = 1:2, v = 3:4)
test(2365.2, dcast(df_dcast, a ~ b, value.var = "v"), dcast(dt_dcast, a ~ b, value.var = "v"))

error: object of type 'symbol' is not subsettable

Will need to test out the point of crash (using gdb) - will check it out tomorrow ^^

[manmita]

Found the issue

The way we discussed to use the following

dcast = function(x, ...) {
  if (!is.data.table(x) && is.data.frame(x)) return(dcast.data.table(x, ...))
  UseMethod("dcast")
}

It was failing at the following lines

 m = as.list(match.call()[-1L])
  subset = m[["subset"]][[2L]]

This is using match call hence m contains data, formula etc meaning m[["subset"]] = subset
hence this was crashing when subset was supposed to be null.

The soln is to use match call at the dcast function and find the user args and forward it and redirect it to dcast.data.table

Although this crash only occured in dcast and a return worked for melt but I have still changed both but I am not sure which will be more optimal perf wise

Let me know if anything needs to be changed or anything can be optimized in this case

Thanks

[MichaelChirico]

Can you confirm this works outside the test suite?

R CMD INSTALL data.table
Rscript -e "library(data.table); dcast(mtcars, cyl ~ gear, value.var='wt', fun.aggregate=mean)"

I am not sure without running it myself whether the frame in which mc is evaluated will be able to see dcast.data.table given that it's not exported.

In the test suite, we define dcast.data.table = data.table:::dcast.data.table, so the test passing is not enough to resolve my apprehension.

[manmita]

> dcast(mtcars, cyl ~ gear, value.var='wt', fun.aggregate=mean)
Key: <cyl>
     cyl        3        4      5
   <num>    <num>    <num>  <num>
1:     4 2.465000 2.378125 1.8265
2:     6 3.337500 3.093750 2.7700
3:     8 4.104083      NaN 3.3700

This is the output for dcast of mtcars
gives the same output on converting it to data.table

> a = as.data.table(mtcars)
> dcast(a, cyl ~ gear, value.var='wt', fun.aggregate=mean)
Key: <cyl>
     cyl        3        4      5
   <num>    <num>    <num>  <num>
1:     4 2.465000 2.378125 1.8265
2:     6 3.337500 3.093750 2.7700
3:     8 4.104083      NaN 3.3700

[manmita]

I had also printed the m values while debugging (line 170 of fcast)
and the results for data.table and data.frame were same after these changes and were as expected
eg: m["data"] was giving dt_dcast which was the user arg passed.

[manmita]

> dcast(mtcars, cyl ~ gear, value.var='wt', fun.aggregate=mean)
$data
mtcars

$formula
cyl ~ gear

$fun.aggregate
mean

$value.var
[1] "wt"

Key: <cyl>
     cyl        3        4      5
   <num>    <num>    <num>  <num>
1:     4 2.465000 2.378125 1.8265
2:     6 3.337500 3.093750 2.7700
3:     8 4.104083      NaN 3.3700
> 

when printing m values

[manmita]

Updated with the suggested changes ^^

[MichaelChirico]

let's do the same as below.

comment about removing this former error

[MichaelChirico]

Thanks!