Skip to content

Unclear class for argument and return value of DT #5559

@OfekShilon

Description

@OfekShilon

While experimenting with a potential fix for #5286 this test of DT surfaced:

  D = copy(mtcars)
...
  test(2212.02, EVAL("D |> DT(,.SD)"), mtcars)

This line in [.data.table :

if (!is.data.table(ans)) setattr(ans, "class", c("data.table","data.frame"))  # DF |> DT(,.SD[...]) .SD should be data.table, test 2212.013

Seems to indicate that the desired behavior is to return a data.table. However ans isn't the variable that is returned directly, later on the code restores the class to data.frame before returning:

The returned object has class data.frame but still has data.table attributes (namely .internal.selfref).

Before suggesting fixes I believe some design decisions need to be explicitly discussed:

  1. Currently DT operates freely on non-data-tables. The only discussion I could find of it is this single comment, and I'm not sure this is the best design. [.data.table, as the name implies, is a method of data.table. It might accidently work in some (even most) scenarios on data.frames, but is a hefty piece of code that was designed to use data.table attributes and trying to open it would mean an avalanche of bugs and a world of pain. In general, as a user I don't expect methods of one class to operate on another - on the contrary, I expect the class system to help me by isolating their respective functionalities.
    In particular, rejecting non-data.table's would make shallow adds .internal.selfref to an input data.frame #5286 (and probably others, both current and future) moot.
  2. Inside [.data.table and other package methods I'd suggest using setDF or setDT instead of setting class directly - as setDF cleans up data.table-only attributes, that manual-class-set misses.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions