Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature Request] Improve error message in case of omitted aggreation #3507

Closed
eddelbuettel opened this issue Apr 15, 2019 · 2 comments · Fixed by #3510
Closed

[Feature Request] Improve error message in case of omitted aggreation #3507

eddelbuettel opened this issue Apr 15, 2019 · 2 comments · Fixed by #3510
Milestone

Comments

@eddelbuettel
Copy link
Contributor

library(data.table)
dt <- data.table(x = runif(100), y = sample(LETTERS[1:5], 100, TRUE))
dt[, .(avg=mean(x), ), by=y]   ## dangling comma past mean()

yields

Error in dotN(this_jsub) : 
  argument "this_jsub" is missing, with no default

whereas 1.11.8 and 1.12.0 produced the somewhat clearer:

Error in list(.External(Cfastmean, x, FALSE), ) : argument 2 is empty

It would be helpful to make the message a little plainer. Took me longer than I care to admit
to realize I left a dangling comma to yield on empty 'operator'.

#  Output of sessionInfo()
R version 3.5.3 (2019-03-11)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Ubuntu 18.10

Matrix products: default
BLAS: /usr/lib/x86_64-linux-gnu/openblas/libblas.so.3
LAPACK: /usr/lib/x86_64-linux-gnu/libopenblasp-r0.3.3.so

locale:
 [1] LC_CTYPE=en_US.UTF-8       LC_NUMERIC=C               LC_TIME=en_US.UTF-8        LC_COLLATE=en_US.UTF-8     LC_MONETARY=en_US.UTF-8    LC_MESSAGES=en_US.UTF-8    LC_PAPER=en_US.UTF-8
 [8] LC_NAME=C                  LC_ADDRESS=C               LC_TELEPHONE=C             LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base

other attached packages:
[1] data.table_1.12.2

loaded via a namespace (and not attached):
[1] compiler_3.5.3
@mattdowle mattdowle added this to the 1.12.4 milestone Apr 15, 2019
@MichaelChirico
Copy link
Member

Quite a strange corner of R we've got here:

x = substitute(list(1, ))[[3L]]
x

Error: argument "x" is missing, with no default

I took a look and I think we should catch this much earlier. Filing PR soon.

@simonfuller79
Copy link

"Took me longer than I care to admit to realize I left a dangling comma to yield on empty 'operator'."
Thanks for the hint! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants