Search registered S3 method for melt before redirecting to reshape2

[odelmarcelle]

Follow-up of #3633 and #3763

The condition leading to UseMethod in the melt generic is not true for non-data.table object. Hence, S3 methods for melt defined by other packages may be skipped.

I initially suggested changing that condition in order to search if a S3 method exists for the current object before redirecting to reshape2.
After giving it a second thought, it appears more correct to performs the redirection to reshape2 in a default method. This leaves the generic clean and allows registering new methods.

Since there were no tests on the generic melt, I left the test file as-is.

After the change, using S3 method dispatch on a custom object works as intended:

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) ## Working fine

[codecov]

Codecov Report

Merging #4864 (293a9e2) into master (9e43317) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 293a9e2 differs from pull request most recent head f06bbc5. Consider uploading reports for the commit f06bbc5 to get more accurate results

@@            Coverage Diff             @@
##           master    #4864      +/-   ##
==========================================
- Coverage   99.54%   99.54%   -0.01%     
==========================================
  Files          76       76              
  Lines       14692    14691       -1     
==========================================
- Hits        14625    14624       -1     
  Misses         67       67              

Impacted Files	Coverage Δ
R/fmelt.R	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9e43317...f06bbc5. Read the comment docs.

[jangorecki]

I think we are now going into new major version so this code could be simplified for raising error, as mentioned in the warning already.

[odelmarcelle]

Do you mean stopping the redirection towards reshape2?

[odelmarcelle]

Committed some additional changes and resolved conflicts. I still think having melt as a generic method makes sense. For example, it could serve to melt an S3 object containing multiple tables and matrices into a single long "melted" format.

I have added a note regarding the change in the NEWS and I added a test on melt.default in test.Rraw.

I think that retaining the redirection towards reshape2 is still useful. As Hadley mentioned in #4908, reshape2 will remain functional for the foreseeable future.

Please let me know if there is anything I missed in this pull request.

[mattdowle]

Thanks @odelmarcelle. Please let me know if your name is different to your id, for the contributor list displayed on CRAN. Have invited you to be project member too: it should be a button in your GitHub profile or projects page that you need to click to accept.

[odelmarcelle]

Thank you for reviewing and merging @mattdowle. Although I'm not sure I deserve to be listed in the contributor list for such a small input, my actual name is different from my Github username.

Could you perhaps correct my name to "Olivier Delmarcelle"? I corrected the DESCRIPTION file on my forked branch, but I'm not sure how to propagate this to the master branch now that this pull request is merged. Sorry for the trouble!

[mattdowle]

Thanks @odelmarcelle, done in 76cfe11.