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

is_integerish doesn't handle NA values consistently #751

Closed
s-fleck opened this issue Mar 31, 2019 · 4 comments

Comments

Projects
None yet
2 participants
@s-fleck
Copy link

commented Mar 31, 2019

is_integerish() handles NA values differently depending on whether they are integers or doubles.

I think the following both lines of code should both return the same value:

> rlang::is_integerish(NA)
[1] FALSE
> rlang::is_integerish(NA_integer_)
[1] TRUE
@lionel-

This comment has been minimized.

Copy link
Member

commented Mar 31, 2019

Agreed, though note that NA is not a double but a logical (try typeof(NA)).

@s-fleck

This comment has been minimized.

Copy link
Author

commented Mar 31, 2019

right, my bad

> rlang::is_integerish(NA_real_)
[1] TRUE

So that behavior for those is consistent. TRUE is not what I would have expected, but I guess there is no "right" decision here.

@lionel-

This comment has been minimized.

Copy link
Member

commented Mar 31, 2019

In general we are now considering NA as having unspecified type, so it makes sense to convert it to na_int here.

@lionel- lionel- closed this in 98ae964 Apr 2, 2019

@lionel- lionel- added the vector label Apr 2, 2019

@lionel-

This comment has been minimized.

Copy link
Member

commented Jun 14, 2019

Sorry, I have reverted this because it caused issues downstream. I think the semantics of rlang are clearer if it deals exclusively with base types, rather than conceptual types. So you'll need to special case for NA. Maybe we could have a conceptual type for integer numbers in the vctrs package (or it could be implemented in another dependent package) to solve this problem, though it's not high priority.

lionel- added a commit to tidyverse/tidyselect that referenced this issue Jun 14, 2019

atusy added a commit to atusy/tidyselect that referenced this issue Jun 24, 2019

atusy added a commit to atusy/tidyselect that referenced this issue Jun 24, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.