jacobkap / fastDummies

The goal of fastDummies is to quickly create dummy variables (columns) and dummy rows.
https://jacobkap.github.io/fastDummies/
Other
36 stars 9 forks source link

Use [.data.table (currently `[` is dispatched to the data.frame() method) #28

Open MichaelChirico opened 2 years ago

MichaelChirico commented 2 years ago

See https://cran.r-project.org/web/packages/data.table/vignettes/datatable-importing.html, relevant section "data.table in Imports but nothing imported"

Basically, because fastDummies Imports data.table but does not importFrom(data.table, ...) in its NAMESPACE, to be conservative data.table assumes that fastDummies is "unaware" of data.table [ semantics and dispatches to [.data.frame.

That's evident e.g. here:

https://github.com/jacobkap/fastDummies/blob/1ff61812f314279706bebec7d4b1c28110f93eba/R/dummy_cols.R#L223

Where that code won't work under [.data.table. The data.table equivalent is:

.data[, !..char_cols]
# or
.data[, !char_cols, with = FALSE]
# or
.data[, .SD, .SDcols = !char_cols]

The solution is

  1. Set .datatable.aware = TRUE anywhere in the package namespace
  2. Edit [ usages to use [.data.table semantics where appropriate.

Happy to file a PR fixing this.

If [.data.frame usage is intentional, I'd recommend (1) using as.data.frame() or data.table::setDF() to make the use of [.data.frame more intentional/explicit or (2) defining .datatable.aware=FALSE in your namespace to make it clearer that this is intentional.

jacobkap commented 2 years ago

Hi, please make a PR. Thanks!