ARROW-14821: [R] Implement bindings for lubridate's floor_date, ceiling_date, and round_date #12154

djnavarro · 2022-01-14T11:37:35Z

This patch provides dplyr bindings to for lubridate functions floor_date(), ceiling_date(), and round_date(). This is my first attempt at writing a patch, so my apologies if I've made any errors 🙂

Supported functionality:

Allows rounding to integer multiples of common time units (second, minutes, days, etc)
Mirrors the lubridate syntax allowing fractional units such as unit = .001 seconds as an alias for unit = 1 millisecond
Allows partial matching of date units based on first three characters: e.g. sec, second, seconds all match second
Mirrors lubridate in throwing errors when unit exceeds thresholds: 60 seconds, 60 minutes, 24 hours

~~### Major problems not yet addressed:~~

~~- Does not yet support the week_start argument, and implicitly fixes week_start = 4~~
~~- Does not yet mirror lubridate handling of timezones~~

~~I'd prefer to fix these two issues before merging, but I'm uncertain how best to handle them. Any advice would be appreciated!~~

~~### Minor things not yet addressed~~

- During rounding lubridate sometimes coerces Date objects to POSIXct. This is not mirrored in the arrow bindings: date32 classes remain date32 classes. This introduces minor differences in rounding in some cases
- Does not yet support the change_on_boundary argument to ceiling_date(). It's a small discrepancy, but it means that the default behaviour of the arrow dplyr binding mirrors lubridate prior to v1.6.0

EDIT: issues now addressed!

github-actions · 2022-01-14T11:49:17Z

https://issues.apache.org/jira/browse/ARROW-14281

github-actions · 2022-01-14T11:49:19Z

⚠️ Ticket has no components in JIRA, make sure you assign one.

rok · 2022-01-14T12:33:22Z

This looks great @djnavarro thanks for doing it!

Regarding your TODOs:

support week_start (essential)

Perhaps you can put in an R shim to get lubridate matching logic and open a Jira to fix this in c++ later on.

support timezones correctly (essential)

I found dates around 1900s quite problematic when testing against Pandas as they don't apply some historic offset changes. I'd suggest aiming at more modern times first.

support change_on_boundary argument to ceiling_date

This would probably be a good c++ Jira candidate too. Provide some examples if you have them handy :).

rok · 2022-01-14T12:41:35Z

Another thing regarding timezones - only timestamps in arrow have timezones. Dates do not.

paleolimbot

Just a few notes! I'm new to creating bindings, too, so take everything with a grain of salt. I noted a few style guide things...I think the linter check will give you notes on this when it runs as well.

paleolimbot · 2022-01-14T12:32:51Z

r/R/util.R

I think if statements elsewhere in arrow R code are always followed by a space (if (...) { ... })

paleolimbot · 2022-01-14T12:39:42Z

r/tests/testthat/test-dplyr-funcs-datetime.R

I think you can test this more directly with:

expect_error( call_binding("round_date", Expression$scalar(Sys.time()), "61 seconds"), "Rounding with second > 60 is not supported" )

Nice! I hadn't realised call_binding() existed 🤦🏻‍♀️ 😁

paleolimbot · 2022-01-14T12:43:04Z

r/tests/testthat/test-dplyr-funcs-datetime.R

I think this would be easier to find at the top of the file next to test_df

paleolimbot · 2022-01-14T12:47:12Z

r/tests/testthat/test-dplyr-funcs-datetime.R

The right way to go, I think!

Yup, I agree here. It might be nice to add a small comment in those test cases about this (so future-us remembers why we have as.Date() at the end there

paleolimbot · 2022-01-14T12:48:12Z

r/R/dplyr-funcs-datetime.R

Suggested change

}

})

paleolimbot · 2022-01-14T12:53:18Z

@jonkeane Can you approve the workflows to run?

ianmcook · 2022-01-14T13:42:16Z

@jonkeane Can you approve the workflows to run?

I approved them

thisisnic · 2022-01-14T15:53:42Z

r/R/util.R

Good catch on these! What do you think about rephrasing a little to say what the user should do, and use the word "must" in there somewhere, kinda like this: https://style.tidyverse.org/error-messages.html ?

(note: feel free to say "no"; we don't do this in every single case)

Good thought. That said, I'm tempted to go with the suggestion @rok makes and allow these cases. It's not inconceivable that people might have a good reason to round to 90 minutes. I can't think of a reason why existing code would depend on this lubridate error?

I can see where you are coming from here. I guess, generally, the aim when implementing the functions for non-standard evaluation code blocks is to (as closely as possible unless there's good reason not to!) replicate the original. However, there are some rare cases when I've seen us not do that; for example, our mapping to lubridate::second() supports nanoseconds as we didn't see the harm in having more precise results, and so the unit test for that binding looks like this:

arrow/r/tests/testthat/test-dplyr-funcs-datetime.R

Lines 474 to 483 in 0c1fd88

test_that("extract second from timestamp", {

compare_dplyr_binding(

.input %>%

mutate(x = second(datetime)) %>%

collect(),

test_df,

# arrow supports nanosecond resolution but lubridate does not

tolerance = 1e-6

)

})

That said, I also can't think of a reason someone else's code would depend on raising an error but I'm not confident that that would mean it won't - people have all sorts of unusual workflows.

There are multiple ways of calling Arrow C++ compute functions from R via the dplyr/non-standard evaluation code - either using the bindings to the lubridate etc equivalent, or calling the Arrow C++ compute function directly by prefixing the function name with arrow_ (so in this case it'd be arrow_round_temporal; if you've not come across this before, check it out here: https://arrow.apache.org/cookbook/r/manipulating-data---tables.html#use-arrow-functions-in-dplyr-verbs-in-arrow).

So if people use that directly they'd still have access to the full functionality. I'm not sure what to suggest here - would be good to get input from @jonkeane or @nealrichardson

That's a really good point. Maybe erring on the side of caution and mirroring lubridate as closely as possible would be best? It's going to come up again as an issue once I try to sort out the issue associated with the change_on_boundary argument, actually

Personally I'd err on the side of caution but I can see the argument for it not mattering - let's see what the more experienced R devs say!

Just to check; what's the issue with change_on_boundary? If it's the fact that it's not supported in the C++ yet, it's fine to open a C++ ticket for that to be implemented, raise an error in the R code saying that parameter is not yet supported, and open an R ticket for implementing it later once the C++ functionality is in place.

Asking over at lubridate

@thisisnic: So yeah... I haven't finished my dive into the lubridate code on the change_on_boundary issue, but (I think!) the TL;DR is that it's an edge case for how ceiling_date() handles time points that sit at the start of a day. Should the ceiling of 2022-01-17 00:00:00 be 2022-01-18 00:00 or 2022-01-17 00:00? Conceptually, "January 17" is an interval of time that starts at 00:00 and ends at 23:59, so a strict interpretation suggests that all time points within that interval should ceiling to midnight of January 18. That's what lubridate currently does, but...

Prior to lubridate 1.6.0 ceiling_date() took a different approach, and AFAICT the C++ round_temporal() function does the same thing. If we were to ignore the strict interpretation of ISO 8601, we can think of "midnight moments" as analogous to integer values spaced along the real line, and round accordingly. Under this approach, which I think is more intuitive but does violate ISO 8601, we don't ceiling an integer up to the next integer, and so 2022-01-17 00:00 stays 2022-01-17 00:00 because it's already an "integer valued date".

The downside to violating ISO 8601 -- in addition to the obvious problem that it violates ISO 8601 -- is that it means we're no longer in agreement with lubridate. The downside to adhering to it is that you get counterintuitive result that taking a ceiling of a Date object always returns the next day:

ceiling_date(ymd("2022-01-17"), "day") # returns 2022-01-18

Long story short, the change_on_boundary argument exists so that lubridate can revert to its older behaviour. Given that the C++ round_temporal() only matches the older lubridate behaviour (again, assuming I've understood the code properly), I'll need to write a shim that supports the change_on_boundary anyway. If I don't we're not mirroring lubridate.

I think the right approach is probably the same one @rok suggested for week_start. Short term the solution is to write a shim to support it on the R side, and open a JIRA ticket for the C++ library

Sounds good!

What y'all have already said is great, and totally agree with this being an edge case for if extending just a bit is fine or if we should error in the same way.

How easy/hard would it be to construct the arrow_round_temporal() call if someone did want to do that?

rok · 2022-01-14T20:10:12Z

r/R/util.R

How about we allow such cases (minute > 60)? C++ implementation supports them and I can imagine I'd want to round to e.g. 90 minutes or 36 hours sometimes.
Users would probably not mind as we would not reduce capabilities.

github-actions · 2022-01-16T21:40:35Z

https://issues.apache.org/jira/browse/ARROW-14821

github-actions · 2022-01-16T21:40:36Z

⚠️ Ticket has not been started in JIRA, please click 'Start Progress'.

djnavarro · 2022-01-17T05:42:40Z

I think I've taken this as far as I can right now? I had been hoping to write shims to support the week_start and change_on_boundary arguments but I'm not sure if that's possible right now. My original thought was to try something like this:

week_start_shim <- function(function_name, x, week_start, opts) {
  offset <- Expression$create(Scalar$create(as.difftime(week_start - 4, units = "days")))
  x <- Expression$create("add", x, offset)
  x <- Expression$create(function_name, x, options = opts)
  return(Expression$create("subtract", x, offset))
}

I can't get anything like this to work because (as far as I can tell) we don't currently have the ability to add or subtract durations from date/time data in Arrow. I imagine I'm missing something obvious here!

rok · 2022-01-17T11:54:43Z

I think I've taken this as far as I can right now? I had been hoping to write shims to support the week_start and change_on_boundary arguments but I'm not sure if that's possible right now. My original thought was to try something like this:
week_start_shim <- function(function_name, x, week_start, opts) {
  offset <- Expression$create(Scalar$create(as.difftime(week_start - 4, units = "days")))
  x <- Expression$create("add", x, offset)
  x <- Expression$create(function_name, x, options = opts)
  return(Expression$create("subtract", x, offset))
} 
I can't get anything like this to work because (as far as I can tell) we don't currently have the ability to add or subtract durations from date/time data in Arrow. I imagine I'm missing something obvious here!

Would it make sense for the shims to be in R?
For add/subtract in arrow see ARROW-11090.

thisisnic · 2022-01-17T20:17:58Z

I think I've taken this as far as I can right now? I had been hoping to write shims to support the week_start and change_on_boundary arguments but I'm not sure if that's possible right now. My original thought was to try something like this:
week_start_shim <- function(function_name, x, week_start, opts) {
  offset <- Expression$create(Scalar$create(as.difftime(week_start - 4, units = "days")))
  x <- Expression$create("add", x, offset)
  x <- Expression$create(function_name, x, options = opts)
  return(Expression$create("subtract", x, offset))
} 
I can't get anything like this to work because (as far as I can tell) we don't currently have the ability to add or subtract durations from date/time data in Arrow. I imagine I'm missing something obvious here!

If the functionality isn't there, I'd be tempted to put this PR aside for the moment - we're aiming to have a larger number of the lubridate and stringr functions implemented for the 8.0.0 release (i.e. ~mid-April) anyway, so there's no rush with this and there's no need to spend ages working around the missing functionality (unless you particularly want to!) when it's totally fine to wait for it to be implemented in C++ anyway. Sorry though, I can imagine that's a little frustrating on the first ticket you've picked up!

djnavarro · 2022-01-17T22:05:56Z

My thinking at this point is that (if this is consistent with the general etiquette here!) I'd like to (1) work out why the build is failing two checks and fix those, (2) merge the passing version, (3) open tickets for the missing functionality, one for the C++ side and one for the R side (and assign myself to the R one 😁 ).

My reasons for suggesting this are:

The current version does work for most use cases. The week_start issue only applies when the rounding unit is "week". As it stands we can round to "7 days" or indeed any time unit except "week" and have it mirror lubridate almost exactly. I've modified the PR so that we abandon ship if the user specifies "week" as the rounding unit, and falls back on lubridate.
It gives me an excuse to do a deeper dive on timezones before merging the current version: if I'm going to write more lubridate bindings later this is something I'm going to need to know anyway!
Once it's merged it would be clearer what modifications are needed to supply the missing functionality, so I can write the JIRA tickets a bit more precisely
A little selfishly... I presume that once I am no longer a first-time contributor I won't need to constantly bug maintainers to enable the GH actions workflows
Slightly less selfishly, it would be mildly convenient for writing blog content about the process. I think there's value in highlighting that a PR can still be useful when surfaces issues pertaining to missing functionality and in doing so immediately triggers new issues?

Possible reasons not to:

As flagged above I'm not sure of the etiquette here. If it's not considered appropriate I'm totally happy to leave this hanging!
The change_on_boundary issue does mean that the Arrow binding for ceiling_date() introduces a minor discrepancy. We're currently mirroring a slightly older lubridate default. I'm not sure that's currently a big issue, but it's something we'd definitely want to revisit before this appears in a release

I'm happy to follow advice here. I'm totally fine with whatever approach more experienced folks think is best! 🙂

thisisnic · 2022-01-17T23:17:02Z

In case it helps, you could try searching for "ARROW-13168" in the test files as there are some tests we skip on Windows because of the issue mentioned in that ticket - not sure if this is related or not though.

rok · 2022-02-14T18:59:45Z

@djnavarro I've opened a jira for the week_start issue.

rok · 2022-02-28T18:06:08Z

@djnavarro week_starts_monday for RoundTemporalOptions was just merged: https://github.com/apache/arrow/blob/master/cpp/src/arrow/compute/api_scalar.h#L107-L120

Does this unblock this PR or do we need to solve change_on_boundary first?

Let me know if I can help somehow.

djnavarro · 2022-03-10T04:19:45Z

@rok sorry for not getting to this earlier!

For the week_start issue: if I'm reading it correctly, we may need a little more flexibility on the C++ side to allow the user to specify an integer between 1 (Monday) and 7 (Sunday). For example, this is the lubridate::floor_date() behaviour:

this_thursday <- ymd("2022-03-10")
floor_date(this_thursday, unit = "week", week_start = 1)  # returns "2022-03-07"
floor_date(this_thursday, unit = "week", week_start = 2)  # returns "2022-03-08"

It's still possible I can handle it at the level of the R binding (I've learned a bit more in the last couple of month!) but I'm wondering if the R level is the best place to do this or if it's better to implement it at the C++ level?

For the change_on_boundary issue: my intuition is that this is best implemented as part of the R binding because the "behaviour of ceiling at the boundary" feels so specific to lubridate? As far as I can tell though it's still blocked because there's no way to add +1 time unit to a temporal object, right? (Or am I missing something???)

rok · 2022-03-10T04:38:58Z

No worries @djnavarro :)
About week_start - are you sure we need anything but 1 and 7 (which maps to week_starts_monday=true/false respectively)? It's relatively straight forward to implement in c++ (and we should probably do it there) but does anyone ever need that? I'm honestly curious. Maybe someone does need it?

About the change_on_boundary - would that be useful for rollup too? See my comment here. Add/subtract kernels can now handle temporal types so you could use them to subtract a single time unit of duration from a timestamp if that's what you'd need.

djnavarro · 2022-03-10T04:50:11Z

Well, I can't think of a good reason why anyone would set week_start to any value other than 1 or 7, but technically lubridate allows it and I suppose there's the possibility that there exists some code that has the week starting on a Wednesday. That being said, if we can now add and subtract durations to temporal types, I should be able to call add_checked() to handle the edge cases, right?

rok · 2022-03-10T05:00:36Z

I'd raise on (week_start != 1 || week_start !=7) just to see who opens the ticket and why do they need this :). On a serious note we can change the c++ logic if we here decide to.

I'm not sure what you mean by edge cases. Could you sketch it with some pseudocode?

djnavarro · 2022-03-10T05:57:08Z

Maybe it's easiest to illustrate with actual R code. The behaviour I need to emulate with regards to the change_on_boundary argument looks different depending on whether the data are stored as a datetime (e.g. POSIXct in R, timestamp in Arrow) versus a Date (e.g., Date in R, date32 in Arrow):

# a datetime object at the midnight boundary, and a date object with no time
midnight_time <- as.POSIXct(strptime("2022-03-10 00:00:00", format = "%F %T"))
timeless_date <- as.Date("2022-03-10")

# ceiling date applied to the datetime
ceiling_date(midnight_time, unit = "day", change_on_boundary = NULL)  # returns the 10th
ceiling_date(midnight_time, unit = "day", change_on_boundary = TRUE)  # returns the 11th
ceiling_date(midnight_time, unit = "day", change_on_boundary = FALSE) # returns the 10th

# ceiling date applied to the date
ceiling_date(timeless_date, unit = "day", change_on_boundary = NULL)  # returns the 11th
ceiling_date(timeless_date, unit = "day", change_on_boundary = TRUE)  # returns the 11th
ceiling_date(timeless_date, unit = "day", change_on_boundary = FALSE) # returns the 10th

This pattern only affects times and dates deemed to be on the boundary with respect to the rounding unit. Any time past midnight and lubridate::ceiling_date() would always return the 11th for the datetime object.

Currently, the ceil_temporal C++ function would return the 10th for the analogous cases. If I can work out how to add +1 time unit to the boundary cases then I have no problems exactly mimicking lubridate behaviour. I can get this to work for timestamp objects. This code works, for instance, and adds 1 day to the Arrow timestamp that corresponds to midnight_time:

call_function("add_checked", Scalar$create(midnight_time), Scalar$create(as.difftime("24:00:00")))

~~But when I try the same thing for the date, however, it throws this error:~~

Error: NotImplemented: Function 'add_checked' has no kernel matching input types (scalar[date32[day]], scalar[duration[s]])

~~That seems odd to me and I'm not sure why I'm getting that error because I'm pretty sure you implemented this in ARROW-14901. So I must be doing something wrong?~~

EDIT: Never mind, I worked it out. 🤦🏻‍♀️ I can handle the change_on_boundary behaviour on the R side!

rok · 2022-03-10T14:01:50Z

That's great news @djnavarro!

Does solving this with subtraction work correctly around DST changes? e.g. (stealing an example from here):

2018-03-25 02:10:00 is in a gap between
2018-03-25 02:00:00 CET and
2018-03-25 03:00:00 CEST which are both equivalent to
2018-03-25 01:00:00 UTC

We should also consider performance implications here. Adding an option to the c++ kernels might be better than calling two different c++ kernels. However that should be a separate ticket.

djnavarro · 2022-03-11T01:54:48Z

@rok Agreed! My thinking at the moment is to see how far I can push this from the R side so that we have a clear idea of exactly what behaviour we're trying to emulate, but we'll certainly want to think about how to improve performance because I can guarantee that the code I'm writing on the R side isn't very efficient.

Maybe the right thing to do is to arrange a voice chat sometime soon?

rok · 2022-03-11T02:30:59Z

@rok Agreed! My thinking at the moment is to see how far I can push this from the R side so that we have a clear idea of exactly what behaviour we're trying to emulate, but we'll certainly want to think about how to improve performance because I can guarantee that the code I'm writing on the R side isn't very efficient.

Perfect. Understanding the behaviour is enough at this stage. And current situation PR is pretty efficient IMO.

…nternal consistency test of timezone-local rounding

…le-unit periods

…test subsecond rounding \facepalm

rok · 2022-07-13T20:28:31Z

@djnavarro I've rebased to fix the C++ conflicts and removed the ARROW-16412 workarounds.
Let me know if another bug pops up :).

djnavarro · 2022-07-21T01:34:17Z

Okay, I think this is actually good? The failing check doesn't appear to be related to this PR as far as I can tell, but notably one of them is timezone related so there might be something going on that I am not quite understanding

2022-07-21T00:50:55.3146284Z ══ Warnings ════════════════════════════════════════════════════════════════════
2022-07-21T00:50:55.3146568Z ── Warning (test-Array.R:269:3): array uses local timezone for POSIXct without timezone ──
2022-07-21T00:50:55.3146749Z running command 'timedatectl' had status 1
2022-07-21T00:50:55.3146829Z Backtrace:
2022-07-21T00:50:55.3146936Z  1. withr::with_envvar(...)
2022-07-21T00:50:55.3147080Z       at test-Array.R:269:2
2022-07-21T00:50:55.3147170Z  6. base (local) `<fn>`()
2022-07-21T00:50:55.3147297Z  7. base::system("timedatectl", intern = TRUE)
2022-07-21T00:50:55.3147562Z ── Warning (test-RecordBatch.R:644:3): Handling string data with embedded nuls ──
2022-07-21T00:50:55.3147887Z Stripping '\0' (nul) from character vector
2022-07-21T00:50:55.3147962Z Backtrace:
2022-07-21T00:50:55.3148060Z   1. withr::with_options(...)
2022-07-21T00:50:55.3148213Z        at test-RecordBatch.R:644:2
2022-07-21T00:50:55.3148314Z   9. arrow:::expect_equal(...)
2022-07-21T00:50:55.3148416Z  10. testthat::expect_equal(...)
2022-07-21T00:50:55.3148679Z        at tests/testthat/helper-expectation.R:42:4
2022-07-21T00:50:55.3148864Z  11. testthat:::expect_waldo_equal("equal", act, exp, info, ..., tolerance = tolerance)
2022-07-21T00:50:55.3148968Z  12. testthat:::waldo_compare(...)
2022-07-21T00:50:55.3149106Z  13. waldo::compare(x, y, ..., x_arg = x_arg, y_arg = y_arg)
2022-07-21T00:50:55.3149267Z  14. waldo:::compare_structure(x, y, paths = c(x_arg, y_arg), opts = opts)
2022-07-21T00:50:55.3149373Z  15. waldo:::is_identical(x, y, opts)
2022-07-21T00:50:55.3149380Z

rok · 2022-07-21T10:35:10Z

This has been observed in other PRs so I think it's not related.

LGTM!

nealrichardson

LGTM, thanks for persisting!

rok · 2022-07-21T22:37:44Z

Thanks for doing this @djnavarro ! 😄

djnavarro · 2022-07-21T22:41:01Z

Woohoo! 🎉

ursabot · 2022-07-22T23:21:35Z

Benchmark runs are scheduled for baseline = 1f03f12 and contender = b0734e6. b0734e6 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Failed ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Failed ⬇️0.17% ⬆️0.0%] test-mac-arm
[Failed ⬇️0.0% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.46% ⬆️0.0%] ursa-thinkcentre-m75q
Buildkite builds:
[Failed] b0734e68 ec2-t3-xlarge-us-east-2
[Failed] b0734e68 test-mac-arm
[Failed] b0734e68 ursa-i9-9960x
[Finished] b0734e68 ursa-thinkcentre-m75q
[Failed] 1f03f126 ec2-t3-xlarge-us-east-2
[Failed] 1f03f126 test-mac-arm
[Failed] 1f03f126 ursa-i9-9960x
[Finished] 1f03f126 ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

…Hub issue numbers (#34260) Rewrite the Jira issue numbers to the GitHub issue numbers, so that the GitHub issue numbers are automatically linked to the issues by pkgdown's auto-linking feature. Issue numbers have been rewritten based on the following correspondence. Also, the pkgdown settings have been changed and updated to link to GitHub. I generated the Changelog page using the `pkgdown::build_news()` function and verified that the links work correctly. --- ARROW-6338 #5198 ARROW-6364 #5201 ARROW-6323 #5169 ARROW-6278 #5141 ARROW-6360 #5329 ARROW-6533 #5450 ARROW-6348 #5223 ARROW-6337 #5399 ARROW-10850 #9128 ARROW-10624 #9092 ARROW-10386 #8549 ARROW-6994 #23308 ARROW-12774 #10320 ARROW-12670 #10287 ARROW-16828 #13484 ARROW-14989 #13482 ARROW-16977 #13514 ARROW-13404 #10999 ARROW-16887 #13601 ARROW-15906 #13206 ARROW-15280 #13171 ARROW-16144 #13183 ARROW-16511 #13105 ARROW-16085 #13088 ARROW-16715 #13555 ARROW-16268 #13550 ARROW-16700 #13518 ARROW-16807 #13583 ARROW-16871 #13517 ARROW-16415 #13190 ARROW-14821 #12154 ARROW-16439 #13174 ARROW-16394 #13118 ARROW-16516 #13163 ARROW-16395 #13627 ARROW-14848 #12589 ARROW-16407 #13196 ARROW-16653 #13506 ARROW-14575 #13160 ARROW-15271 #13170 ARROW-16703 #13650 ARROW-16444 #13397 ARROW-15016 #13541 ARROW-16776 #13563 ARROW-15622 #13090 ARROW-18131 #14484 ARROW-18305 #14581 ARROW-18285 #14615 * Closes: #33631 Authored-by: SHIMA Tatsuya <ts1s1andn@gmail.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>

github-actions bot added the Component: R label Jan 14, 2022

paleolimbot reviewed Jan 14, 2022

View reviewed changes

thisisnic reviewed Jan 14, 2022

View reviewed changes

rok reviewed Jan 14, 2022

View reviewed changes

thisisnic changed the title ~~ARROW-14281: [R] Implement bindings for lubridate's floor_date, ceiling_date, and round_date~~ ARROW-14821: [R] Implement bindings for lubridate's floor_date, ceiling_date, and round_date Jan 16, 2022

github-actions bot added Component: C++ Component: Python labels Mar 30, 2022

djnavarro added 5 commits July 13, 2022 21:37

removes trailing whitespace

b114276

adds missing test: timezone check for round to week

c6cf2b1

fixes date strings test bug

109e46d

bypasses esoteric lubridate timezone-local rounding bug and adds an i…

6310abb

…nternal consistency test of timezone-local rounding

adds internal consistency tests for timezone-local rounding to multip…

a9b3144

…le-unit periods

rok force-pushed the ARROW-14281 branch from 148c885 to 0fdc3f3 Compare July 13, 2022 19:40

sigh. don't test subsecond rounding with test that isn't designed to …

16f989b

…test subsecond rounding \facepalm

rok force-pushed the ARROW-14281 branch from 0fdc3f3 to 16f989b Compare July 13, 2022 19:54

Removing ARROW-16412 workarounds

bbbbe00

djnavarro added 7 commits July 21, 2022 09:31

incorporate stylistic comments

9a402c7

moves temporal rounding helpers to dplyr-datetime-helpers.R

dbf6a31

tidies style on helper functions

5962dcb

more stylistic edits

14b74e0

removes outdated comment regarding ARROW-16142

76b3c73

notes what we mean by an "easy" date

13ae009

simplifies second/multiple check

075fcfc

rok requested a review from nealrichardson July 21, 2022 10:35

nealrichardson approved these changes Jul 21, 2022

View reviewed changes

nealrichardson merged commit b0734e6 into apache:master Jul 21, 2022

This was referenced Jul 22, 2022

[R] Implement bindings for lubridate's floor_date, ceiling_date, and round_date #30353

Closed

[C++] Temporal floor/ceil/round should accept week_starts_monday when rounding to multiple of week #31134

Closed

eitsupi mentioned this pull request Feb 19, 2023

GH-33631: [R] Rewrite Jira ticket numbers in pkgdown documents to GitHub issue numbers #34260

Merged

	test_that("extract second from timestamp", {
	compare_dplyr_binding(
	.input %>%
	mutate(x = second(datetime)) %>%
	collect(),
	test_df,
	# arrow supports nanosecond resolution but lubridate does not
	tolerance = 1e-6
	)
	})

ARROW-14821: [R] Implement bindings for lubridate's floor_date, ceiling_date, and round_date #12154

ARROW-14821: [R] Implement bindings for lubridate's floor_date, ceiling_date, and round_date #12154

Uh oh!

Conversation

djnavarro commented Jan 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Supported functionality:

Uh oh!

github-actions bot commented Jan 14, 2022

Uh oh!

github-actions bot commented Jan 14, 2022

Uh oh!

rok commented Jan 14, 2022

Uh oh!

rok commented Jan 14, 2022

Uh oh!

paleolimbot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paleolimbot commented Jan 14, 2022

Uh oh!

ianmcook commented Jan 14, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rok Jan 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jan 16, 2022

Uh oh!

github-actions bot commented Jan 16, 2022

Uh oh!

djnavarro commented Jan 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rok commented Jan 17, 2022

Uh oh!

thisisnic commented Jan 17, 2022

Uh oh!

djnavarro commented Jan 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thisisnic commented Jan 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rok commented Feb 14, 2022

Uh oh!

rok commented Feb 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

djnavarro commented Jan 14, 2022 •

edited

Loading

rok Jan 14, 2022 •

edited

Loading

djnavarro commented Jan 17, 2022 •

edited

Loading

djnavarro commented Jan 17, 2022 •

edited

Loading

thisisnic commented Jan 17, 2022 •

edited

Loading

rok commented Feb 28, 2022 •

edited

Loading

djnavarro commented Mar 10, 2022 •

edited

Loading