⚠ This page is served via a proxy. Original site: https://github.com
This service does not collect credentials or authentication data.
Skip to content

Conversation

@manmita
Copy link
Contributor

@manmita manmita commented Jan 27, 2026

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

@manmita manmita marked this pull request as draft January 27, 2026 20:59
@github-actions
Copy link

github-actions bot commented Jan 27, 2026

  • HEAD=fix/dcast_melt_for_data.frame stopped early for DT[by,verbose=TRUE] improved in #6296
  • HEAD=fix/dcast_melt_for_data.frame stopped early for DT[,.SD] improved in #4501
    Comparison Plot

Generated via commit a05b069

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

Task Duration
R setup and installing dependencies 2 minutes and 51 seconds
Installing different package versions 23 seconds
Running and plotting the test cases 4 minutes and 5 seconds

R/fcast.R Outdated
dcast.data.frame = function(data, ...) {
if (!is.data.frame(data)) stopf("'%s' must be a data.frame", "data")
data = as.data.table(data)
dcast.data.table(data, ...)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it

@manmita manmita marked this pull request as ready for review January 27, 2026 21:17
R/fmelt.R Outdated
ans
}

melt.data.frame = function(data, ...) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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")
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link

codecov bot commented Jan 27, 2026

Codecov Report

❌ Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 99.01%. Comparing base (b537ec4) to head (6502e86).
⚠️ Report is 5 commits behind head on master.

Files with missing lines Patch % Lines
R/fcast.R 66.66% 1 Missing ⚠️
R/fmelt.R 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7634      +/-   ##
==========================================
- Coverage   99.02%   99.01%   -0.02%     
==========================================
  Files          87       87              
  Lines       16890    16896       +6     
==========================================
+ Hits        16726    16730       +4     
- Misses        164      166       +2     

☔ 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
Copy link
Contributor Author

manmita commented Jan 27, 2026

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

prolly due to fast subsetting in data tables which isnt present in data frames?
Will need to test out the point of crash (using gdb) - will check it out tomorrow ^^

@manmita manmita changed the title feat(7614): added s3 method at dcast and melt for data.frame feat(7614): added redirection of data.frame at dcast and melt to dcast.data.table and melt.data.table Jan 27, 2026
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 this pull request may close these issues.

Can dcast/melt "just work" with data.frame input?

2 participants