Skip to content

Conversation

@rok
Copy link
Member

@rok rok commented Aug 3, 2022

Changes introduced by ARROW-16653 were not written up by NEWS.md.

Copy link
Contributor

@dragosmg dragosmg left a comment

Choose a reason for hiding this comment

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

A minor point

r/NEWS.md Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Are there any formats we still do not support? With the exception of locale-specific ones on Windows?

Copy link
Member Author

Choose a reason for hiding this comment

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

strftime supports: %a %A %w %d %b %B %m %y %Y %H %I %p %M %z %Z %j %U %W %x %X %% %G %V %u
strptime on Windows supports: "%d", "%H", "%j", "%m", "%T", "%S", "%q", "%M", "%U", "%w", "%W", "%y", "%Y", "%R", "%T"
strptime on non-Windows in additionally supports: "%a", "%A", "%b", "%B", "%Om", "%OS", "%I", "%p", "%r", "%z"
parse_date_time supports on Windows: "H", "I", "j", "M", "S", "U", "w", "W", "y", "Y", "R", "T", "%H", "%I", "%j", "%M", "%S", "%U", "%w", "%W", "%y", "%Y", "%R", "%T"
parse_date_time on non-Windows supports: "a", "A", "b", "B", "Om", "p", "r", "%a", "%A", "%b", "%B", "O%m", "%p", "%r"

I'm not sure where to put this information :)

Copy link
Contributor

@dragosmg dragosmg Aug 4, 2022

Choose a reason for hiding this comment

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

Would this statement be accurate : "for parse_date_time() we support all formats on Linux and macOS. On windows we don't support ... "? And we stick to parse_date_time() only?

Copy link
Member

Choose a reason for hiding this comment

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

@rok could you instead add a note to the binding (see #14387) that explains which tokens are not supported (since it seems like most everything is)? Or make a note that punts on the whole question like the base R docs to and just say "support is platform dependent" and mostly identical to R.

@paleolimbot
Copy link
Member

Just browsing some old PRs before we're in a release rush...is this PR still relevant/is there anything I can do to put it over the line?

@rok
Copy link
Member Author

rok commented Jan 8, 2023

Hey @paleolimbot I'll take a look tomorrow!

@rok rok force-pushed the MINOR-update-NEWS.md-regarding-parse_date_time branch from a9781da to 50e3200 Compare January 16, 2023 15:30
@rok
Copy link
Member Author

rok commented Jan 16, 2023

@paleolimbot sorry this took a while to get to. Please check if this make sense.

@rok rok requested a review from paleolimbot January 16, 2023 15:49
Copy link
Member

@paleolimbot paleolimbot left a comment

Choose a reason for hiding this comment

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

Thanks!

(Just waiting for CI to do its thing)

@paleolimbot paleolimbot merged commit 611409e into apache:master Jan 16, 2023
@ursabot
Copy link

ursabot commented Jan 17, 2023

Benchmark runs are scheduled for baseline = 7492cbe and contender = 611409e. 611409e 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 ⬇️0.18% ⬆️0.03%] test-mac-arm
[Finished ⬇️0.0% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️1.03% ⬆️0.12%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] 611409e5 ec2-t3-xlarge-us-east-2
[Failed] 611409e5 test-mac-arm
[Finished] 611409e5 ursa-i9-9960x
[Finished] 611409e5 ursa-thinkcentre-m75q
[Finished] 7492cbea ec2-t3-xlarge-us-east-2
[Failed] 7492cbea test-mac-arm
[Finished] 7492cbea ursa-i9-9960x
[Finished] 7492cbea 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

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants