ENH: infer Timestamp unit in non-iso paths #51039
Conversation
MarcoGorelli
added
the
Non-Nano
| def test_constructor_str_infer_reso(self): | ||
| # non-iso8601 path | ||
| ts = Timestamp("2016-01-01 1:30:01 PM") | ||
| assert ts.unit == "s" | ||
|
|
||
| ts = Timestamp("2016 June 3 15:25:01.345") | ||
| assert ts.unit == "ms" |
There was a problem hiding this comment.
I think we need 3 tests here, one for each path inside parse_datetime_string:
dt = _parse_delimited_date(date_string, dayfirst, out_bestunit)_parse_dateabbr_string(dt = dateutil_parse(date_string, default=_DEFAULT_DATETIME,
| # non-iso8601 path | ||
|
|
||
| # _parse_delimited_date path | ||
| ts = Timestamp("2023/01/30") |
There was a problem hiding this comment.
this hits the iso path, you'll need
ts = Timestamp("01/30/2023")
also, would prefer to parametrise these - you can put _parse_delimited_date path (and the others) as id for the different cases
There was a problem hiding this comment.
updated to use the suggested string. didnt parametrize - on the margin id like to slow the growth of the pytest overhead
There was a problem hiding this comment.
just out of interest, why?
There was a problem hiding this comment.
pytest collects something like 200k tests and last time i checked that added up to more than 1GB before we even start executing any tests. Similarly autouse fixtures get called for each test which means that much more overhead at runtime (i haven't measured this so assume it is a small-but-adds-up amount).
Doing a little bit more inside the tests and a little bit less outside can trim that footprint and overhead. Adding a couple cases here isn't a big deal, but on the margin i try to get in the habit of leaning against parametrization when it isn't a compelling use case.
doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.