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.

Sign up for GitHub

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

Jump to bottom

Helpful error could extend to when the symbol is a function #5014

Closed
MichaelChirico opened this issue May 20, 2021 · 4 comments · Fixed by #5021
Closed

Helpful error could extend to when the symbol is a function #5014

MichaelChirico opened this issue May 20, 2021 · 4 comments · Fixed by #5021
Milestone
1.15.0

Comments

@MichaelChirico
Copy link
Member

We already have a helpful error for accidentally treating a column as a table:

DT = data.table(vv = runif(100) > .5)
DT[vv]
# Error in `[.data.table`(DT, vv) : 
#  vv is not found in calling scope but it is a column of type logical. If you wish to select rows where that column contains TRUE, or perhaps that column contains row numbers of itself to select, try DT[(col)], DT[DT$col], or DT[col==TRUE] is particularly clear and is optimized. When the first argument inside DT[...] is a single symbol (e.g. DT[var]), data.table looks for var in calling scope.

However this doesn't appear if the culprit column happens to mask a function:

setnames(DT, 'vv', 'filter')
DT[filter]
# Error in `[.data.table`(DT, filter) : 
#  i has evaluated to type closure. Expecting logical, integer or double.

It would be nice if it's easy to extend the helpful message to such a case.
@MichaelChirico
Copy link
Member Author

I'm not sure there's an easy way to do this unfortunately -- the friendly message relies on eval(isub, parent.frame(), parent.frame()) failing, which won't happen in the case of this issue since eval() will find the call symbol. Best we could do is to put this alongside the other check:

exists(as.character(isub), envir=parent.frame(), mode="function")

Any opinion on whether it'd be useful enough to add this check?

@ColeMiller1
Copy link
Contributor

It would be helpful. Wouldn’t implementation be similar to #4660? E.g if (is.function(i)) stop(...)

@MichaelChirico
Copy link
Member Author

Unfortunately not:

is.function(substitute(filter))
# [1] FALSE

Seems we have to get() the name to find its type.

@ColeMiller1
Copy link
Contributor

There are two error messages we are discussing. The second error message of

i has evaluated to type ...

happens after isub has been evaluated. Based on the title of the issue, I would recommend my suggestion.

The first error gets to the calling scope. I still believe i should look in the data.table before looking in the parent.frame. Expected behavior of example is to use the logical vector to subset DT. See also #4255

I do not believe it would be worth checking get for this issue. However, we could combine approaches and if isub evaluated to a function, we can do additional checks to see if it was a column name. e.g.

if (is.function(i))
  if (is.name(isub) && exists(as.character(isub), ...)
else
   ...

That way there’s no additional overhead for this infrequent error but users would still get a helpful error.

@MichaelChirico MichaelChirico mentioned this issue May 23, 2021
Merged
@mattdowle mattdowle added this to the 1.14.1 milestone May 23, 2021
@jangorecki jangorecki modified the milestones: 1.14.9, 1.15.0 Oct 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.15.0
Development

Successfully merging a pull request may close this issue.

detect a function-cum-column in i Rdatatable/data.table
4 participants
@mattdowle @jangorecki @MichaelChirico @ColeMiller1