dimnames.data.table short use inherits, not identical

[MichaelChirico]

Currently, internal dimnames.data.table requires the class to be c("data.table", "data.frame"), but when users are mixing with the Hadleyverse, their data.table might end up with other classes tacked on as well.

See this SO post, for example, where class(dt) ended up as c("grouped_dt", "tbl_dt", "tbl", "tbl_dt", "tbl", "data.table", "data.frame") (kind of silly, but it's beyond our control).

[jangorecki]

This issue is not related just to that method, having other classes as leading results into multiple issues because of dispatching, there was discussion on that somewhere in issues

[MichaelChirico]

just to be sure I understand, you mean this issue is nested within another, broader one?

[jangorecki]

Yes, R won't use data.table method for a function if dplyr assign it's own class that has the same method. Unfortunately dplyr methods don't always dispatch to data.table methods. Related #1161, #1114, #1078

[MichaelChirico]

@jangorecki got it. but check out my PR for this issue -- it seems this problem in particular is on the data.table end and has a quick fix.

[arunsrinivasan]

Could you provide a reproducible example of the issue? I don't follow what the issue is or what the fix does. Specifically, why do you check if an object is a data.frame inside a function that's a S3 method for objects that inherits data.table?

[MichaelChirico]

@arunsrinivasan Check the SO post; they don't really provide an example, but my understanding is a data.table is being fed to a (data.table-unaware) package which calls dim, maybe shiny?

The issue is a data.table-unaware package (hence !cedta()) has called dim on a dplyr-churned data.table (so it's acquired extra elements in class). dim.data.table just redirects to dim.data.frame in this case, but there's a test (must be years old) in there to make sure dim.data.frame is OK to use.

The original approach of the test was too strict -- requiring anything sent to dim.data.table to have class exactly c("data.table", "data.frame"). I think the original idea of that test, which I again think must be years old, was to error if the class match wasn't exact, because at that time the most common source of this error was a data.table that had class data.table only.

The fix in my PR is to only test that the data.table is a data.frame and prevent any issues before using a data.frame method.

Hope that's clear enough. I couldn't reproduce it myself.

Now I'm thinking that we could probably just skip that test altogether, and just send straight to dim.data.frame, no more questions asked.

[arunsrinivasan]

Thanks. The issue seems to be checking for exact class. It should be !is.data.table(x) instead. Have commented there.

[MichaelChirico]

@arunsrinivasan I'm not sure that's exactly right. I'm leaning towards ditching that test altogether; see my comment below yours.