TYP: libperiod #40990
TYP: libperiod #40990
Conversation
jbrockmendel
commented
Apr 16, 2021
jbrockmendel
commented
- closes #xxxx
- tests added / passed
- Ensure all linting tests pass, see here for how to run them
- whatsnew entry
pandas/core/arrays/datetimelike.py
Outdated
| new_fill = self._scalar_type(fill_value) | ||
| # error: Incompatible types in assignment (expression has type | ||
| # "Union[Period, Any, Timedelta]", variable has type "Period") | ||
| new_fill = self._scalar_type(fill_value) # type: ignore[assignment] |
There was a problem hiding this comment.
new_fill is inferred as Period from the first assignment to new_fill above. https://mypy.readthedocs.io/en/stable/type_inference_and_annotations.html#type-inference
remove ignore by adding add new_fill: DatetimeLikeScalar before initial assignment
| # error: Argument 1 of "_unbox_scalar" is incompatible with supertype | ||
| # "DatetimeLikeArrayMixin"; supertype defines the argument type as | ||
| # "Union[Union[Period, Any, Timedelta], NaTType]" | ||
| def _unbox_scalar( # type: ignore[override] |
There was a problem hiding this comment.
I think DatetimeLikeArrayMixin needs to be Generic[DatetimeLikeScalarT].
There was a problem hiding this comment.
Trying this i get error: Free type variable expected in Generic[...] [misc]
I'll troubleshoot this a bit
There was a problem hiding this comment.
ok to leave as a 'fix later' ignore so we can start benefitting straight away from the added types
| if value is NaT: | ||
| return np.int64(value.value) | ||
| # error: Item "Period" of "Union[Period, NaTType]" has no attribute "value" | ||
| return np.int64(value.value) # type: ignore[union-attr] |
There was a problem hiding this comment.
This is a false positive. if not planning to 'fix later', can either cast or add a reference to a mypy issue.
maybe python/mypy#7642, although AFAICT the discussion there is more around a singleton class (i.e. final, not subclassed) rather than a singleton instance.
however, from python/mypy#7642 (comment)
You're trying to introduce a singleton value other than None, and you want the type checker to understand code that excludes the singleton value.
I think this is a reasonable request.
so probably an appropriate issue to discuss this.
| @@ -910,7 +918,7 @@ def raise_on_incompatible(left, right): | |||
|
|
|||
|
|
|||
| def period_array( | |||
| data: Sequence[Period | None] | AnyArrayLike, | |||
| data: Sequence[Period | str | None] | AnyArrayLike, | |||
There was a problem hiding this comment.
and from the docstring pd.NaT (np.nan also seems to be acceptable but not pd.NA) and also datetime.datetime
>>> period_array(
... [pd.Period("2017", freq="A"), pd.Period("2018", freq="A"), datetime.datetime.now()]
... )
<PeriodArray>
['2017', '2018', '2021']
Length: 3, dtype: period[A-DEC]
>>>
pandas/_libs/tslibs/period.pyx
Outdated
| @@ -2539,7 +2539,7 @@ cdef int64_t _ordinal_from_fields(int year, int month, quarter, int day, | |||
| minute, second, 0, 0, base) | |||
|
|
|||
|
|
|||
| def validate_end_alias(how): | |||
| def validate_end_alias(how: str) -> str: | |||
There was a problem hiding this comment.
returns Literal["S", "E"] or raises?
| ) -> np.ndarray: ... # np.ndarray[np.int64] | ||
|
|
||
| def get_period_field_arr( | ||
| field: str, |
There was a problem hiding this comment.
do we need a alias to a Literal for this to match accessor type is the cython code.
pandas/_libs/tslibs/period.pyi
Outdated
|
|
||
| def from_ordinals( | ||
| values: np.ndarray, # const int64_t[:] | ||
| freq, |
There was a problem hiding this comment.
str, tuple, datetime.timedelta, DateOffset or None
same as type of freq to to_offset in pandas/_libs/tslibs/offsets.pyx, maybe need an alias since used in several places
pandas/_libs/tslibs/period.pyi
Outdated
|
|
||
| def extract_ordinals( | ||
| values: np.ndarray, # np.ndarray[object] | ||
| freq, |
There was a problem hiding this comment.
same as above and also int?
pandas/_libs/tslibs/period.pyi
Outdated
|
|
||
| def strftime(self, fmt: str) -> str: ... | ||
|
|
||
| def to_timestamp(self, freq=..., how=..., tz=...) -> Timestamp: ... |
There was a problem hiding this comment.
missed types on this one.
pandas/_libs/tslibs/period.pyi
Outdated
| @property | ||
| def start_time(self) -> Timestamp: ... | ||
|
|
||
| def __sub__(self, other): ... |
There was a problem hiding this comment.
annotating as Period for now. for the Technically Correct longer term, do we annotate that it can return NotImplemented?
There was a problem hiding this comment.
sure. I don't think NotImplemented is a valid type.
(when mypy is checking the function NotImplemented needs to be compatible with any return type to avoid false positives, but that's not relevant for stubs)
|
Thanks @jbrockmendel for the updates. some new mypy failures. |
|
mypy complaints fixed; new CI failure is 3.10, looks unrelated |
|
thanks @jbrockmendel |
