ARROW-15802 [R] bindings for lubridate::make_datetime() and lubridate::make_date()

[dragosmg]

• bindings for lubridate::make_datetime() and lubridate::make_date()
• bindings for base::ISOdatetime() and base::ISOdate()

[github-actions]

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

[jonkeane]

This is looking good. A few comments about structure + tests.

Looking at the docs for these, it looks like our implementation of make_datetime() could also be mapped to base:ISOdate() and base::ISOdatetime()

[dragosmg]

The are some outstanding issues, mostly having to do with the timezone argument:

1. default values:

• for ISOdate() the default value is "GMT" which, in fact, is "UTC".
• for ISOdatetime() the default value is "" (current timezone)
• for lubridate::make_datetime() the default value is "UTC"

2. the strptime binding does not support the tz argument - pointing to ARROW-12820 - which is now solved. I have a feeling that issue doesn't actually mean we can now switch on tz support in strptime.

For 1, the solution could be to set the default value for all tz args to "UTC".

[jonkeane]

The are some outstanding issues, mostly having to do with the timezone argument:

1. default values:
   for ISOdate() the default value is "GMT" which, in fact, is "UTC".
   I think this is fine, we can make ours just be "UTC" it's most important to match behavior even if it means our signatures are slightly different.

for ISOdatetime() the default value is "" (current timezone)
This is similar to our other issue with needing to add a TZ in arrow if we have "". Can we leave it as is and will that automagically become fixed when that other ticket is done? Speaking of: what is the status of that other ticket?

for lubridate::make_datetime() the default value is "UTC"
This is ok, right?

2. the strptime binding does not support the tz argument - pointing to ARROW-12820 - which is now solved. I have a feeling that issue doesn't actually mean we can now switch on tz support in strptime.
   Yes, though for this binding you can actually "just" specify the timezone after you do the strptime, no?

For 1, the solution could be to set the default value for all tz args to "UTC".
Yup!

[dragosmg]

Speaking of: what is the status of that other ticket?

Still blocked by the Windows tz database availability issue #12536.

[dragosmg]

This is similar to our other issue with needing to add a TZ in arrow if we have "". Can we leave it as is and will that automagically become fixed when that other ticket is done?

Not sure that will automagically be solved since strptime will still not accept a tz argument (and that is the barrier).

[dragosmg]

Yes, though for this binding you can actually "just" specify the timezone after you do the strptime, no?

Not sure I follow, once x is created, I don't think we can change the value of x$type$timezone (it seems to be locked) without casting.

[jonkeane]

This looks good. Thanks!

[ursabot]

Benchmark runs are scheduled for baseline = acda3c6 and contender = de02cfc. de02cfc 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.38% ⬆️0.0%] test-mac-arm
[Failed ⬇️1.07% ⬆️0.71%] ursa-i9-9960x
[Finished ⬇️0.17% ⬆️0.04%] 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