API: pd.Timedelta(integer, unit=unit) give the requested unit #63302
Conversation
divya1974
left a comment
divya1974
left a comment
There was a problem hiding this comment.
Some points to consider:
- Confirm cast_from_unit accepts the new out_reso argument (or update it).
- Replace magic numeric bounds with named constants (or compute from datetime.timedelta.max/min) and add a short comment.
- Add tests that check very large integer Timedelta inputs for weeks/days/hours/minutes (overflow edges) and tests that NaT values stay preserved."
| with pytest.raises(OutOfBoundsDatetime, match="10155196800000000000"): | ||
| pd.to_timedelta(106580, "D") + Timestamp("2000") | ||
| with pytest.raises(OutOfBoundsDatetime, match="10155196800000000000"): | ||
| Timestamp("2000") + pd.to_timedelta(106580, "D") |
There was a problem hiding this comment.
You can keep those if you make the timestamp(or timedelta) ns with adding a .as_unit("ns") ?
| (lambda x: timedelta(days=x), "m8[us]"), | ||
| (lambda x: Timedelta(x, "D"), "m8[ns]"), | ||
| (lambda x: Timedelta(x, "D"), "m8[s]"), | ||
| (lambda x: Timedelta(x, "D").as_unit("s"), "m8[s]"), |
There was a problem hiding this comment.
| (lambda x: Timedelta(x, "D").as_unit("s"), "m8[s]"), | |
| (lambda x: Timedelta(x, "D").as_unit("ms"), "m8[ms]"), |
Otherwise it is the same as the case above now. Or just remove the case alltogether
| with tm.assert_produces_warning(Pandas4Warning, match=msg): | ||
| result = read_json(StringIO(ser.to_json()), typ="series").apply(converter) | ||
| tm.assert_series_equal(result, ser) | ||
| tm.assert_series_equal(result, ser.astype("m8[ms]")) |
There was a problem hiding this comment.
Not at all related to your change, but wondering: do you know why we apply this converter instead of checking the actual return value of read_json? Because applying to_timedelta on the result (and which output change now) has nothing to do with the reading of json .. (I assume the reading actually returns strings)
|
|
||
|
|
||
| class TestTimedeltaConstructorUnitKeyword: | ||
| def test_result_unit(self): |
There was a problem hiding this comment.
Do you have a similar explicit test for pd.to_timedelta ?
There was a problem hiding this comment.
Ah, that's because to_timedelta is not yet updated in this PR. I assumed it was based on some changed tests, but that was only for the case of a scalar input to to_timedelta, not array-like input
3 participants
Matching the Timestamp behavior. Part of #63275. Sits on top of #63301.