Skip to content

ARROW-14848: [R] Implement bindings for lubridate's parse_date_time - #12589

Closed
dragosmg wants to merge 47 commits into
apache:masterfrom
dragosmg:parse_date_time
Closed

ARROW-14848: [R] Implement bindings for lubridate's parse_date_time#12589
dragosmg wants to merge 47 commits into
apache:masterfrom
dragosmg:parse_date_time

Conversation

@dragosmg

@dragosmgdragosmg commented Mar 9, 2022

Copy link
Copy Markdown
Contributor

This PR adds a partial implementation of parse_date_time():

  • only parses the year, month, and date components (no hours, minutes and seconds yet)
  • does not support parsing of strings without separators (e.g. "220912" to 2022-09-12)
  • lubridate::parse_date_time() infers the most likely format given orders (via guess_formats(), while the Arrow binding does not do any inference.

@github-actions

Copy link
Copy Markdown

Thanks for opening a pull request!

If this is not a minor PR. Could you open an issue for this pull request on JIRA? https://issues.apache.org/jira/browse/ARROW

Opening JIRAs ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename pull request title in the following format?

ARROW-${JIRA_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

See also:

@dragosmgdragosmg changed the title first pass at implementing fast_strptime()ARROW-14848 [R] Implement bindings for lubridate's parse_date_time, parse_date_time2, and fast_strptimeMar 9, 2022
@github-actions

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown

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

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

Copy link
Copy Markdown
ContributorAuthor

There seems to be a registration issue with replace_substring_regex: https://github.com/apache/arrow/runs/5787187215?check_suite_focus=true#step:20:18491

@dragosmg
dragosmg marked this pull request as ready for review April 1, 2022 14:44
Comment threadr/R/dplyr-funcs-datetime.R Outdated
@dragosmgdragosmg changed the title ARROW-14848 [R] Implement bindings for lubridate's parse_date_time, parse_date_time2, and fast_strptimeARROW-14848: [R] Implement bindings for lubridate's parse_date_time, parse_date_time2, and fast_strptimeApr 28, 2022
@dragosmg

Copy link
Copy Markdown
ContributorAuthor

It looks like the arrow replace_substring_regex function (used by the gsub binding) isn't registered for R 3.6 with rtools35 on Windows. Would it be acceptable to skip the offending unit test on R 3.6 on Windows?

@dragosmg

dragosmg commented May 6, 2022

Copy link
Copy Markdown
ContributorAuthor

I added a detailed NEWS entry about the current limitations of parse_date_time().

@dragosmg
dragosmg marked this pull request as draft May 6, 2022 11:18
@dragosmg
dragosmg marked this pull request as ready for review May 6, 2022 11:23
@dragosmg
dragosmg marked this pull request as draft May 6, 2022 11:29
@dragosmg
dragosmg marked this pull request as ready for review May 6, 2022 11:49
@dragosmg

Copy link
Copy Markdown
ContributorAuthor

@jonkeane is there anything else I need to do on this PR?

Comment threadr/NEWS.md Outdated
@dragosmg
dragosmg requested a review from jonkeaneMay 9, 2022 15:07
@dragosmg

Copy link
Copy Markdown
ContributorAuthor

@jonkeane is there anything else needed for this PR?

Comment threadr/NEWS.md Outdated
@ursabot

Copy link
Copy Markdown

Benchmark runs are scheduled for baseline = b264dca and contender = 214135d. 214135d 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
[Finished ⬇️0.58% ⬆️0.0%] test-mac-arm
[Finished ⬇️2.5% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.32% ⬆️0.04%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] 214135d8 ec2-t3-xlarge-us-east-2
[Finished] 214135d8 test-mac-arm
[Finished] 214135d8 ursa-i9-9960x
[Finished] 214135d8 ursa-thinkcentre-m75q
[Finished] b264dca5 ec2-t3-xlarge-us-east-2
[Finished] b264dca5 test-mac-arm
[Finished] b264dca5 ursa-i9-9960x
[Finished] b264dca5 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.
ursa-i9-9960x

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.

6 participants

@dragosmg@thisisnic@amol-@jonkeane@ursabot@jorisvandenbossche