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

handling i which is a factor #1632

Closed
aushev opened this issue Apr 6, 2016 · 6 comments · Fixed by #5120
Closed

handling i which is a factor #1632

aushev opened this issue Apr 6, 2016 · 6 comments · Fixed by #5120

Comments

@aushev
Copy link

aushev commented Apr 6, 2016

It happens quite often to me that column which I passed as i, turns out to be a factor. Current behavior is to throw an error: "i has not evaluated to logical, integer or double", so I have to manually convert it to the same class as the key of the target table.

  1. One option would be just to add this conversion to "[.data.table" function code. I guess there are some reasons against as it have not been done yet, would be glad to hear it.
  2. When it happened first time, it took me some time to figure out what was the problem. It would be helpful to make error message more informative, just add class(i) at the end of message in stop() call.
@MichaelChirico
Copy link
Member

Related to #1621 -- seems we'd do well to give better errors when factors are involved

@aushev
Copy link
Author

aushev commented Apr 6, 2016

well, for me, it's not the same situation there - it is not processing of i... (but I see your point in general)

@jangorecki
Copy link
Member

Are you converting to integer or character (factor levels)?
I think both types (integer and character) could be expected in such case, so it may be better to keep conversion on the user side. Any reason why not to convert factors into character in the first place, if you are going to use them as character?

@aushev
Copy link
Author

aushev commented Apr 6, 2016

I am converting it to the same class as the key of the data.table I am calling - usually it's character. It just happens that the table where I take i from has it as a factor (usually just because it was created this way, but sometimes I explicitly want it to be factor, for other operations). OK, I take the point of suggesting user to make conversion - but still believe that it would not hurt to add this info to error message (show specific message if is.factor(i), or generally add class(i) to that message).

@jangorecki
Copy link
Member

Agree, we need to add check for is.factor, current error is little bit confusing.

setkey(data.table(a=factor("a")))[factor("a")]
#Error in `[.data.table`(setkey(data.table(a = factor("a"))), factor("a")) : 
#  i has evaluated to type integer. Expecting logical, integer or double.

@mattdowle
Copy link
Member

mattdowle commented Aug 30, 2021

This was unfortunate. The workaround was in fact to just wrap factor f with .(); i.e DT[.(f)] so that a join would be invoked.
#5120 means that workaround is no longer needed and DT[f] just works.

image

@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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants