API: microsecond resolution for Timedelta strings #63196
Conversation
jorisvandenbossche
added
Timedelta
| # TODO: more performant way of doing this check? | ||
| if ival % 1000 != 0: | ||
| return True | ||
| return re.search(r"\.\d{9}", item) or "ns" in item or "nano" in item |
There was a problem hiding this comment.
Is the 9 correct here? Shouldn't 7 be enough to have something below microseconds?
(although it is not entirely clear what this is exactly used for, will first check the rest of the PR, but some additional docstring might be useful)
There was a problem hiding this comment.
Ah, is this meant for something that ends on 0's to still infer as nanosecond?
i.e.
>>> pd.Timedelta("1.123456000 seconds").unit
'ns'
i.e. that could be microseconds (no loss of data), but because the string included the nanos, we infer as nanosecond unit?
There was a problem hiding this comment.
I suppose 7 digits should then also still count as nanosconds?
Right now we get:
>>> pd.Timedelta("1.1234561 seconds").unit
'ns'
>>> pd.Timedelta("1.1234560 seconds").unit
'us'
| def test_groupby_timedelta_median(): | ||
| # issue 57926 | ||
| expected = Series(data=Timedelta("1D"), index=["foo"]) | ||
| expected = Series(data=Timedelta("1D"), index=["foo"], dtype="m8[ns]") |
There was a problem hiding this comment.
I was wondering why ns, but it is the other PR that will preserve the unit of the Timedelta object when converting to an array?
| "ufunc 'divide' cannot use operands", | ||
| "Invalid dtype object for __floordiv__", | ||
| r"unsupported operand type\(s\) for /: 'int' and 'str'", | ||
| r"unsupported operand type\(s\) for /: 'datetime.timedelta' and 'str'", |
There was a problem hiding this comment.
Just curious, how is this caused by the changes here?
| assert Timedelta(10.0, unit="D")._value == expected | ||
| assert Timedelta("10 days")._value == expected | ||
| assert Timedelta("10 days")._value == expected // 1000 | ||
| assert Timedelta(days=10)._value == expected |
There was a problem hiding this comment.
Just to keep track, this is another code path that should also still be updated ideally? (for another PR)
Similarly as doing Timedelta(datetime.timedelta()), which already gives us
| assert td.unit == "us" | ||
|
|
||
| td = Timedelta("1 Day 2:03:04.012345123") | ||
| assert td.unit == "ns" |
There was a problem hiding this comment.
| assert td.unit == "ns" | |
| assert td.unit == "ns" | |
| td = Timedelta("1 Day 2:03:04.012345000") | |
| assert td.unit == "ns" |
3 participants
doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.AGENTS.md.Ideally we'd do the check for needs_nano_unit at a lower level and avoid effectively re-parsing the string, but the existing parse_timedelta_string function is a tough nut to crack.