Skip to content

ARROW-16516: [R] Implement ym() my() and yq() parsers - #13163

Closed
dragosmg wants to merge 11 commits into
apache:masterfrom
dragosmg:ym_my_yq_parsers
Closed

ARROW-16516: [R] Implement ym() my() and yq() parsers#13163
dragosmg wants to merge 11 commits into
apache:masterfrom
dragosmg:ym_my_yq_parsers

Conversation

@dragosmg

Copy link
Copy Markdown
Contributor

The ym(), my() and yq() bindings will make the following possible (and identical):

library(arrow, warn.conflicts=FALSE)
library(dplyr, warn.conflicts=FALSE)
library(lubridate, warn.conflicts=FALSE)
test_df<-tibble::tibble(
ym_string= c("2022-05", "2022/02", "22.03", NA)
)
test_df %>% mutate(ym_date= ym(ym_string))
#> # A tibble: 4 × 2#> ym_string ym_date #> <chr> <date> #> 1 2022-05 2022-05-01#> 2 2022/02 2022-02-01#> 3 22.03 2022-03-01#> 4 <NA> NAtest_df %>% arrow_table() %>% mutate(ym_date= ym(ym_string)) %>% collect()
#> # A tibble: 4 × 2#> ym_string ym_date #> <chr> <date> #> 1 2022-05 2022-05-01#> 2 2022/02 2022-02-01#> 3 22.03 2022-03-01#> 4 <NA> NA

Created on 2022-05-16 by the reprex package (v2.0.1)

I've implementing this with the following steps:

  • add "-01" to the end of the strings we're trying to parse, and then
  • use one the supported orders ("ymd" or "myd")

@dragosmg
dragosmg marked this pull request as ready for review May 16, 2022 10:27
@dragosmg
dragosmg marked this pull request as draft May 16, 2022 10:34
@github-actions

Copy link
Copy Markdown

@dragosmg
dragosmg marked this pull request as ready for review May 16, 2022 14:12

@thisisnicthisisnic left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good - chuck in a few explanatory comments and I'll take another look.

Comment threadr/R/dplyr-datetime-helpers.R
Comment threadr/R/dplyr-datetime-helpers.R
Comment threadr/R/dplyr-datetime-helpers.R
Comment threadr/R/dplyr-funcs-datetime.R
Comment threadr/R/dplyr-funcs-datetime.R
@dragosmg
dragosmg requested a review from thisisnicMay 17, 2022 12:59
Comment threadr/tests/testthat/test-dplyr-funcs-datetime.R

@thisisnicthisisnic left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, but would like a second pairs of eyes on this before merging; @jonkeane or @paleolimbot - mind taking a look?

Comment threadr/R/dplyr-datetime-helpers.R Outdated
@thisisnic
thisisnic requested a review from jonkeaneMay 17, 2022 16:55

@paleolimbotpaleolimbot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't been following the latest PRs on this front and I think it's very cool what you've been able to do! Just two notes about for() + out[[i] <- something possibly being rewritten using a map()-like expression (but no strong feelings if we do this elsewhere in Arrow and I missed it).

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

Copy link
Copy Markdown

Benchmark runs are scheduled for baseline = 0742f78 and contender = 60f6caf. 60f6caf 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 ⬇️2.06% ⬆️1.48%] test-mac-arm
[Failed ⬇️0.0% ⬆️11.03%] ursa-i9-9960x
[Finished ⬇️1.38% ⬆️0.28%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] 60f6caf9 ec2-t3-xlarge-us-east-2
[Failed] 60f6caf9 test-mac-arm
[Failed] 60f6caf9 ursa-i9-9960x
[Finished] 60f6caf9 ursa-thinkcentre-m75q
[Finished] 0742f78a ec2-t3-xlarge-us-east-2
[Failed] 0742f78a test-mac-arm
[Failed] 0742f78a ursa-i9-9960x
[Finished] 0742f78a 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

@ursabot

Copy link
Copy Markdown

['Python', 'R'] benchmarks have high level of regressions.
test-mac-arm

kou pushed a commit that referenced this pull request Feb 20, 2023
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@dragosmg@ursabot@paleolimbot@thisisnic