ARROW-15800 [R] Implement bindings for `lubridate::as_date()` and `lubridate::as_datetime()` by dragosmg · Pull Request #12738 · apache/arrow

dragosmg · 2022-03-28T20:27:56Z

No description provided.

github-actions · 2022-03-28T21:11:45Z

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

…es on Windows

…from epoch

AlenkaF

Looks good! Some comments added to clarify things for me. Thanks!

…e()` supports `format`

… `as.Date()` + unit test for `format` + not skipping tests on windows since we now have tzdb

dragosmg · 2022-04-13T12:11:12Z

@AlenkaF I think this is ready for another look. Thanks!

AlenkaF

LGTM, thank you for the changes!

dragosmg · 2022-04-14T06:56:43Z

Thanks @AlenkaF. @thisisnic @jonkeane @nealrichardson would you be able to have a look and merge if everything is ok?

jonkeane

Thanks, a few comments + questions for you

…`int32()`

dragosmg · 2022-04-20T14:02:08Z

@jonkeane I think this is ready for another look.

jonkeane

This is so so so close! A few minor style things, and a few requests for some comments.

dragosmg · 2022-04-22T08:56:54Z

I think the CI fails are unrelated to the changes in the PR

jonkeane · 2022-04-22T16:08:40Z

I think the CI fails are unrelated to the changes in the PR

It's good practice to enumerate which are failing + link to the issues that are resolving them, or that they are failing on master, etc.

Both the R / AMD64 Ubuntu 20.04 R 4.1 build and the linter should be resolved by #12945 or #12958

ursabot · 2022-04-25T20:22:23Z

Benchmark runs are scheduled for baseline = c4b646e and contender = 16638a4. 16638a4 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Failed] test-mac-arm
[Failed ⬇️0.38% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.29% ⬆️0.0%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/587|16638a44 ec2-t3-xlarge-us-east-2>
[Failed] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/575|16638a44 test-mac-arm>
[Failed] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/573|16638a44 ursa-i9-9960x>
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/585|16638a44 ursa-thinkcentre-m75q>
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/586|c4b646e7 ec2-t3-xlarge-us-east-2>
[Failed] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/574|c4b646e7 test-mac-arm>
[Failed] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/572|c4b646e7 ursa-i9-9960x>
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/584|c4b646e7 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

dragosmg mentioned this pull request Mar 28, 2022
ARROW-15800 [R] bindings for lubridate::as_date() and lubridate::as_datetime() - abandoned #12677
Closed
2 tasks

dragosmg commented Mar 28, 2022
View reviewed changes

Comment threadr/R/dplyr-funcs-type.R Outdated

dragosmg commented Mar 28, 2022
View reviewed changes

Comment threadr/tests/testthat/test-dplyr-funcs-type.R Outdated

dragosmg commented Mar 28, 2022
View reviewed changes

Comment threadr/R/dplyr-funcs-type.R Outdated

github-actionsBot added the Component: R label Mar 28, 2022

dragosmg added 5 commits March 29, 2022 10:19

as.Date() supports integer-like inputs
3c0bd20

as_date() bindings + tests + NEWS
e24de04

tests for as_datetime() plus skipped another test involving timezon…
122865e
…es on Windows

as_datetime() binding
3cc8365

removed tz from the casting to timestamp step
3431636

dragosmg force-pushed the as_date_as_datetime_take2 branch from f78c583 to 3431636Compare March 29, 2022 09:19

dragosmg added 6 commits March 29, 2022 14:05

simplified
fd9ee4a

updated unit tests as I have added support for an origin different …
ba1bee2
…from epoch

added abstract function a level below as.Date() and as_date()
28a12a9

single {testthat} block for as_date() and as.Date()
bfbd3b7

tidy-up
48b498e

some more tidying up
af69bf0

dragosmg marked this pull request as ready for review March 29, 2022 15:27

AlenkaF reviewed Apr 5, 2022
View reviewed changes

Comment threadr/R/dplyr-funcs-type.R Outdated
Comment threadr/R/dplyr-funcs-datetime.R

dragosmg added 4 commits April 13, 2022 12:28

Merge branch 'master' into as_date_as_datetime_take2
dd9ea68

default value for tz with as_date() is now NULL and `as_datetim…
2fbba5f
…e()` supports `format`

support for NULLtz in binding_as_date()
9b3d6c4

added unit test to better show the difference between as_date() and…
97f59b4
… `as.Date()` + unit test for `format` + not skipping tests on windows since we now have tzdb

AlenkaF approved these changes Apr 13, 2022
View reviewed changes

jonkeane requested changes Apr 18, 2022
View reviewed changes

Comment threadr/R/dplyr-funcs-datetime.R Outdated
Comment threadr/R/dplyr-funcs-datetime.R Outdated
Comment threadr/R/dplyr-funcs-datetime.R Outdated
Comment threadr/tests/testthat/test-dplyr-funcs-type.R Outdated
Comment threadr/tests/testthat/test-dplyr-funcs-type.R Outdated

dragosmg added 2 commits April 20, 2022 10:58

remove self-asignment
fd8ba19

colon
110fa15

dragosmg added 3 commits April 20, 2022 11:02

no longer skipping on Windows
02c4df2

separated the double and origin different from "1970-01-01" logic
9a1b0ec

revert to casting delta_in_sec to int64() and delta_in_days to …
634863e
…`int32()`

dragosmg requested a review from jonkeane April 20, 2022 14:58

dragosmg added 3 commits April 20, 2022 17:07

Merge branch 'master' into as_date_as_datetime_take2
0820b0e

rename tz -> use_tz and added some clarifying comments
054d926

removed x no-op and added comments
f2aa904

jonkeane reviewed Apr 20, 2022
View reviewed changes

Comment threadr/R/dplyr-funcs-datetime.R Outdated

dragosmg added 5 commits April 21, 2022 09:24

docs
5a258b9

remove the base and tz arguments
7576460

removed comments referencing ARROW-13168
1b45fda

added comments on the as.Date() vs as_date() differece
7281395

refactored binding_as_date()
c1ed1cc

dragosmg requested a review from jonkeane April 21, 2022 10:16

jonkeane requested changes Apr 21, 2022
View reviewed changes

Comment threadr/R/dplyr-funcs-datetime.R
Comment threadr/R/dplyr-funcs-datetime.R
Comment threadr/tests/testthat/test-dplyr-funcs-type.R
Comment threadr/R/dplyr-funcs-datetime.R
Comment threadr/R/dplyr-funcs-datetime.R Outdated

dragosmg added 3 commits April 22, 2022 09:16

added links to relevant Jiras
833c432

added comment on why we're not testing the content of the error message
1ab2e2d

style
06b7f86

dragosmg requested a review from jonkeane April 22, 2022 11:28

edit comment
3e721b4

jonkeane closed this in 16638a4Apr 22, 2022

dragosmg deleted the as_date_as_datetime_take2 branch April 22, 2022 16:38

asfimport mentioned this pull request Apr 28, 2022
[R] Implement bindings for lubridate::as_date() and lubridate::as_datetime() #31242
Closed

Uh oh!

Conversation

dragosmg commented Mar 28, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actionsBot commented Mar 28, 2022

Uh oh!

AlenkaF left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dragosmg commented Apr 13, 2022

Uh oh!

AlenkaF left a comment

Choose a reason for hiding this comment

Uh oh!

dragosmg commented Apr 14, 2022

Uh oh!

jonkeane left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dragosmg commented Apr 20, 2022

Uh oh!

Uh oh!

jonkeane left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dragosmg commented Apr 22, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonkeane commented Apr 22, 2022

Uh oh!

ursabot commented Apr 25, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dragosmg commented Apr 22, 2022•
edited
Loading