Skip to content

floor_date() does not preserve NA on R devel #1069

Description

@nealrichardson

Ran into this very fine edge case in apache/arrow#14282 (comment). Due to a recent change in R devel (my guess is wch/r-source@4f70ce0), floor_date() can erroneously return 1970-01-01 for an NA Date input.

# Current R:
> lubridate::floor_date(as.Date(NA), "1 day")
[1] NA

# R at commit 82963
> lubridate::floor_date(as.Date(NA), "1 day")
[1] "1970-01-01"

I dug a bit and saw that it has to do with lubridate:::floor_multi_unit1(): on NA input, it returns NA_real_ instead instead of the integer type that belongs in all of the fields of a POSIXlt object. This used to be fine:

# Current R
> as.Date(structure(list(sec =  0L, min =  0L, hour =  0L, mday = NA_real_, 
+     mon = NA_integer_, year = NA_integer_, wday = NA_integer_, 
+     yday = NA_integer_, isdst = -1L), class = c("POSIXlt", "POSIXt"
+ ), tzone = "UTC"))
[1] NA

But in R devel, it no longer does what is expected:

> as.Date(structure(list(sec =  0L, min =  0L, hour =  0L, mday = NA_real_, 
+     mon = NA_integer_, year = NA_integer_, wday = NA_integer_, 
+     yday = NA_integer_, isdst = -1L), class = c("POSIXlt", "POSIXt"
+ ), tzone = "UTC"))
[1] "1970-01-01"

though if mday were integer and not real, you get the expected NA result:

> as.Date(structure(list(sec =  0L, min =  0L, hour =  0L, mday = NA_integer_, 
+     mon = NA_integer_, year = NA_integer_, wday = NA_integer_, 
+     yday = NA_integer_, isdst = -1L), class = c("POSIXlt", "POSIXt"
+ ), tzone = "UTC"))
[1] NA

Arguably, R should handle this correctly. Happy to report this upstream if you think that's best. Though one could also argue that sticking something that's not an integer in a POSIXlt is breaking the data contract. (That said, I didn't see anywhere in the R docs that said all fields had to be integers, and to support fractional seconds, it would have to accept numeric at least in secs.)

Either way, it seems like this could be easily worked around in lubridate by having floor_multi_unit1() wrap its output in as.integer(). Happy to submit a PR for this if you'd like.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions