dtplyr
dtplyr copied to clipboard
tidyverse
Ensure code executed by dtplyr is marked as data.table aware
When called from within another package, dt_eval() ends up with a root environment that's the other package's namespace (which might not import data.table). It would be nice if dtplyr always worked without intervention by the package author. But cedta() always calls topenv() so it's not clear how to tell data.table that this specific call is ok (even if other calls from the same package aren't data.table aware).
See discussion in https://github.com/ianmcook/tidyquery/issues/17
@MichaelChirico any ideas?
Another option would to to set option(topLevelEnvironment) to the evaluation environment and then set .datatable.aware <- TRUE in that env. It's not clear what else might be affected by changing the topLevelEnvironment, but it is unlikely to affect code typically seen in a data manipulation pipeline.
if the package is expecting some data.table functionality, defining .datatable.aware <- TRUE seems like the right way to go, no? In my mind it's like an even more dotted-line version of Suggests.
@MichaelChirico but the package isn't necessarily using data.table, it's using dtplyr. dtplyr is definitely using data.table, but has no way to communicate that to data.table.
But if relying on dtplyr, certainly that author is aware of data.table, so they can signal that to data.table with .datatable.aware... what you're going for is that dtplyr "just works" without needing this little kludge, right?
Another option would to to set option(topLevelEnvironment) to the evaluation environment and then set .datatable.aware <- TRUE in that env. It's not clear what else might be affected by changing the topLevelEnvironment, but it is unlikely to affect code typically seen in a data manipulation pipeline.
Could dtplyr check isNamespace in the calling environments and unlock them, then assign .datatable.aware if it's not already defined? (Possibly with warning...)
@MichaelChirico right, it would be nice if dtplyr could declare this without requiring an hacks. I'd really prefer not to be writing into other people's namespaces. That said, if you don't think there's interest in resolving this on the data.table side, I can either document as a requirement for package authors or implement my own kludge.
Could you file a PR citing this issue adding dtplyr here:
"data.table" %chin% names(imports <- getNamespaceImports(ns)) ||
"dtplyr" %chin% names(imports) ||
What about also checking for .datatable.aware in the evaluation environment? Something like:
env = parent.frame(n)
if (isTRUE(env$.datatable.aware)) {
return(TRUE)
}
ns = topenv(env)
That also looks doable. I would probably set .datatable.aware=TRUE inside the data.table namespace as well since we aim for that case to be handled first.
I'm not sure all of the intricacies at play here (in fact I am not sure we even need cedta() any more), the PR would help flesh that out. Thanks!