RFC, feat: infer datetime format for pyarrow backend

[FBruzzesi]

What type of PR is this? (check all applicable)

• 💾 Refactor
• ✨ Feature
• 🐛 Bug Fix
• 🔧 Optimization
• 📝 Documentation
• ✅ Test
• 🐳 Other

Related issues

• Closes [Enh]: Infer datetime format for pyarrow backend in to_datetime(format=None) case #1151

Checklist

• Code follows style guide (ruff)
• Tests added
• Documented the changes

If you have comments or can explain your changes, please do so below.

As discussed in the comment of the PR tagged in the issue, the implementation is inspired by and adjusted from the following repository: datetime-format.

I am opening this as draft and looking for help for:

• Enlarge the test suite with multiple formats, if it would be possible to do this dynamically (maybe with hypothesis it would be great - I never used it though)
• Add other common formats:
  • allow for AM/PM
  • Are there other date separators?
  • Are there other time separators?
  • other?

For the two latter questions, sometimes at work we use no separators e.g. YYYYmmddHHMMSS

PS: CI Failure on doctest is unrelated

[MarcoGorelli]

ooh, nice!

I think this might already be a good start? if we can at least auto-detect iso8601-like formats, that's already an improvement

[MarcoGorelli]

thanks for doing this!

If I understand correctly, this looks a bit too expensive - we're checking whether each element matches a given format, trying this for multiple formats, and then picking one

This is more expensive that what pandas does, which is just infer the format from the first non-null element, and then use that for all elements

Could we just do that instead?

[MarcoGorelli]

just tried timing this, and noticed a couple of things:

• '%Y-%m-%dT%H:%M' doesn't get inferred. I think we should auto-infer this one too
• the overhead is quite significant:

In [27]: s = pd.Series(pd.date_range('2000', periods=100_000, freq='h')).dt.strftime('%Y-%m-%dT%H:%M:%S')

In [28]: s_pa = pa.chunked_array([s])

In [29]: %time nw.from_native(s_pa, series_only=True).str.to_datetime(format='%Y-%m-%dT%H:%M:%S').to_native()
CPU times: user 5.89 ms, sys: 0 ns, total: 5.89 ms
Wall time: 5.94 ms
Out[29]: 
<pyarrow.lib.ChunkedArray object at 0x7fe623eafe80>
[
  [
    2000-01-01 00:00:00.000000,
    2000-01-01 01:00:00.000000,
    2000-01-01 02:00:00.000000,
    2000-01-01 03:00:00.000000,
    2000-01-01 04:00:00.000000,
    ...
    2011-05-29 11:00:00.000000,
    2011-05-29 12:00:00.000000,
    2011-05-29 13:00:00.000000,
    2011-05-29 14:00:00.000000,
    2011-05-29 15:00:00.000000
  ]
]

In [30]: %time nw.from_native(s_pa, series_only=True).str.to_datetime().to_native()
CPU times: user 111 ms, sys: 9.42 ms, total: 121 ms
Wall time: 130 ms
Out[30]: 
<pyarrow.lib.ChunkedArray object at 0x7fe62865ca00>
[
  [
    2000-01-01 00:00:00.000000,
    2000-01-01 01:00:00.000000,
    2000-01-01 02:00:00.000000,
    2000-01-01 03:00:00.000000,
    2000-01-01 04:00:00.000000,
    ...
    2011-05-29 11:00:00.000000,
    2011-05-29 12:00:00.000000,
    2011-05-29 13:00:00.000000,
    2011-05-29 14:00:00.000000,
    2011-05-29 15:00:00.000000
  ]
]

[FBruzzesi]

If I understand correctly, this looks a bit too expensive - we're checking whether each element matches a given format, trying this for multiple formats, and then picking one

This is more expensive that what pandas does, which is just infer the format from the first non-null element, and then use that for all elements

Could we just do that instead?

If I take the slice of the first 10 elements, then performances seem to not be impacted - clearly 10 is arbitrary number I tried for a test.

'%Y-%m-%dT%H:%M' doesn't get inferred. I think we should auto-infer this one too

Sure we can increase the supported formats, I just wanted to put the PR out to make sure that the approach is reasonable

[MarcoGorelli]

sure, first 10 seems fine, thanks!

and we can leave ''%Y-%m-%dT%H:%M'' for later

[MarcoGorelli]

wonderful, thanks @FBruzzesi ! looks good on green

let's make a release and update the plotly PR?

[FBruzzesi]

Something is off with some tests, I will take a look

[FBruzzesi]

@MarcoGorelli should be good to go now :)

[MarcoGorelli]

amazing, love this