-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
between() function is not generic #3744
Comments
@leungi where can I find tsibbledata ? Alternatively could you make a reproducible example with dummy data ? This seems to work fine for me: > tibble( index = ymd("2014-03-01") ) %>%
+ filter(between(index, ymd("2014-03-01"), ymd("2014-03-04")))
# A tibble: 1 x 1
index
<date>
1 2014-03-01 |
Thanks for prompt reply @romainfrancois.
|
Isn't this a tsibble issue? |
I think the date-time issue is related to Here's the reprex. library(lubridate)
#>
#> Attaching package: 'lubridate'
#> The following object is masked from 'package:base':
#>
#> date
x <- make_datetime(2018) + hours(0:48)
dplyr::between(x, ymd("2018-01-01"), ymd("2018-01-03"))
# expected as x >= ymd("2018-01-01") & x <= ymd("2018-01-03")
#> [1] FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE
#> [12] FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE
#> [23] FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE
#> [34] FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE
#> [45] FALSE FALSE FALSE FALSE FALSE
x >= ymd("2018-01-01") & x <= ymd("2018-01-03")
#> [1] TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE
#> [15] TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE
#> [29] TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE
#> [43] TRUE TRUE TRUE TRUE TRUE TRUE TRUE
dplyr::between(x, ymd("2018-01-01"), ymd_h("2018-01-03 0")) # repaired version
#> [1] TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE
#> [15] TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE
#> [29] TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE
#> [43] TRUE TRUE TRUE TRUE TRUE TRUE TRUE Created on 2018-08-11 by the reprex |
@earowang, thanks for investigation! I was going to follow up this weekend, but you got ahead 👍 |
I think the cause for all this is because Line 20 in dd9b2b9
Now, as date/datetime object can be converted to numeric, it should work. However, Date object and datetime object (POSIXct) are not the same and cannot be compared as numeric. When providing date or datetime to I detailed the behavior and the different option in the reprex below library(lubridate)
# this is a datetime object (POSIXct)
x <- make_datetime(2018, 01, 02, 10) # 2018-01-02 10:00 UTC
class(x)
#> [1] "POSIXct" "POSIXt"
as.numeric(x)
#> [1] 1514887200
# ymd with tz = NULL (the default) returns a Date object
y <- ymd("2018-01-01", tz = NULL)
class(y)
#> [1] "Date"
as.numeric(y)
#> [1] 17532
# between converts to numeric and is right to return false
dplyr::between(x, ymd("2018-01-01", tz = NULL), ymd("2018-01-03", tz = NULL))
#> [1] FALSE
as.numeric(x) >= as.numeric(ymd("2018-01-01", tz = NULL)) & as.numeric(x) <= as.numeric(ymd("2018-01-03", tz = NULL))
#> [1] FALSE
# lubridate::ymd with tz not NULL returns a datetime object (POSIXct)
class(ymd("2018-01-01", tz = "UTC"))
#> [1] "POSIXct" "POSIXt"
# it works fine if all are the same type
dplyr::between(x, ymd("2018-01-01", tz = "UTC"), ymd("2018-01-03", tz = "UTC"))
#> [1] TRUE
as.numeric(x) >= as.numeric(ymd("2018-01-01", tz = "UTC")) & as.numeric(x) <= as.numeric(ymd("2018-01-03", tz = "UTC"))
#> [1] TRUE
# you could also use lubridate::as_datetime
class(as_datetime("2018-01-01"))
#> [1] "POSIXct" "POSIXt"
# it works fine if all are the same type
dplyr::between(x, as_datetime("2018-01-01"), as_datetime("2018-01-03"))
#> [1] TRUE Created on 2018-08-12 by the reprex package (v0.2.0). So, I think there is no bug here - it is just how In your case with
Hope it helps. |
Since this is a problem related to types/classes it will eventually be addressed in vctrs, not dplyr. I think there’s already an issue about between in vctrs; it’d be great if someone could find it and cross link with this issue |
Also related to #2995 |
@cderv, thanks for the detailed review; points taken. A few notes on my end. Without a timestamp in string,
By specifying any component of timestamp,
Converting to |
Closing this in favor of https://github.com/r-lib/vctrs/issues/147 |
This old issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with reprex) and link to this issue. https://reprex.tidyverse.org/ |
The
tsibble
package importsdplyr::filter()
.The text was updated successfully, but these errors were encountered: