TYP: some type annotations in core\tools\datetimes.py #34630
Conversation
pandas/tests/series/test_ufunc.py
Outdated
| pd.to_datetime(["2000", "2010", "2001"]).tz_localize("CET"), # type: ignore | ||
| pd.to_datetime(["2000", "2010", "2001"]).to_period(freq="D"), # type: ignore |
There was a problem hiding this comment.
$ mypy pandas
pandas\tests\series\test_ufunc.py:258: error: "DatetimeIndex" has no attribute "tz_localize"
pandas\tests\series\test_ufunc.py:259: error: "DatetimeIndex" has no attribute "to_period"
Found 2 errors in 1 file (checked 1018 source files)
There was a problem hiding this comment.
IIRC this happens bc we use inherit_names on DatetimeIndex. I'd be OK with pinning the inherited methods in a way mypy gets even if its a little more verbose if you think its worthwhile
pandas/core/tools/datetimes.py
Outdated
|
|
||
| @overload | ||
| def to_datetime( | ||
| arg: Union[list, tuple], |
There was a problem hiding this comment.
For consistency in code base can you use List, Tuple here?
There was a problem hiding this comment.
good catch. cut and paste from the alias. will update the alias too.
pandas/core/tools/datetimes.py
Outdated
| exact=None, | ||
| format: Optional[str], | ||
| name: Label = None, | ||
| tz: Optional[str] = None, |
There was a problem hiding this comment.
tz could be a tzinfo? might merit a type in pd._typing corresponding to "tzinfo or str that we can convert to one"
pandas/core/tools/datetimes.py
Outdated
| infer_datetime_format: bool = ..., | ||
| origin=..., | ||
| cache: bool = ..., | ||
| ) -> Timestamp: |
There was a problem hiding this comment.
The return type here is dependent of the value of the errors parameter. so to refine the overload further requires Literal.
updated to a Union return type with NaT but needs a cast (or ignore/assert) in pandas/io/excel/_odfreader.py
also changed from Timestamp to DatetimeScalar since with error="ignore" returns arg value
There was a problem hiding this comment.
so to refine the overload further requires Literal
FWIW I think (separate to this PR) that we will need to start vendoring parts of typing_extensions and this would be a good candidate. I also think Protocol is necessary for us to unpin where we are at now particularly with our mixin classes
There was a problem hiding this comment.
+1 for vendoring typing_extensions
| @@ -746,8 +809,7 @@ def to_datetime( | |||
| if not cache_array.empty: | |||
| result = _convert_and_box_cache(arg, cache_array, name=arg.name) | |||
| else: | |||
| convert_listlike = partial(convert_listlike, name=arg.name) | |||
| result = convert_listlike(arg, format) | |||
There was a problem hiding this comment.
+1 for avoiding the partial here
No description provided.