TYP: aggregations.pyx #41029
TYP: aggregations.pyx #41029
Conversation
| self.min_periods, | ||
| # error: Argument 4 to "ewmcov" has incompatible type | ||
| # "Optional[int]"; expected "int" | ||
| self.min_periods, # type: ignore[arg-type] |
There was a problem hiding this comment.
The EWM subclass ensures this is set to int before passing to the parent class which has Optional[int] typing
| @@ -1068,7 +1071,10 @@ def sum(self, *args, **kwargs): | |||
| def mean(self, *args, **kwargs): | |||
| nv.validate_window_func("mean", args, kwargs) | |||
| window_func = window_aggregations.roll_weighted_mean | |||
| return self._apply(window_func, name="mean", **kwargs) | |||
| # error: Argument 1 to "_apply" of "Window" has incompatible type | |||
There was a problem hiding this comment.
EWM doesn't subclass Window so this can be ignored
pandas/_libs/window/aggregations.pyi
Outdated
| end: np.ndarray, # np.ndarray[np.int64] | ||
| minp: int, # int64_t | ||
| quantile: float, # float64_t | ||
| interpolation: str, |
There was a problem hiding this comment.
could use Literal here?
pandas/_libs/window/aggregations.pyi
Outdated
| start: np.ndarray, # np.ndarray[np.int64] | ||
| end: np.ndarray, # np.ndarray[np.int64] | ||
| minp: int, # int64_t | ||
| function: object, |
There was a problem hiding this comment.
callable? Callable[..., Any] used in _generate_cython_apply_func
pandas/_libs/window/aggregations.pyi
Outdated
| raw: bool, | ||
| args: tuple, | ||
| kwargs: dict, | ||
| ) -> np.ndarray: ... # np.ndarray[float] |
There was a problem hiding this comment.
the code starts with
if n == 0:
return obj
so the return type could be same as obj
pandas/_libs/window/aggregations.pyi
Outdated
| args: tuple, | ||
| kwargs: dict, |
There was a problem hiding this comment.
These are passed to func with no restrictions so I think OK to type as
| args: tuple, | |
| kwargs: dict, | |
| args: tuple[Any, ...], | |
| kwargs: dict[str, Any], |
to be consistent with _generate_cython_apply_func
pandas/core/window/ewm.py
Outdated
| self.min_periods, | ||
| # error: Argument 4 to "ewmcov" has incompatible type | ||
| # "Optional[int]"; expected "int" | ||
| self.min_periods, # type: ignore[arg-type] |
There was a problem hiding this comment.
in the outer scope we have
min_periods = (
self.min_periods
if self.min_periods is not None
else window_indexer.window_size
)
can we not just pass min_periods here.
There was a problem hiding this comment.
True; since the EWM constructor always has an numeric min_periods this can just be self.min_periods
|
Thanks @jbrockmendel lgtm pending green. |
|
updated + green |
|
Thanks @jbrockmendel |
* TYP: aggregations.pyx * update per comments
* TYP: aggregations.pyx * update per comments
@mroeschke the type:ignores added in ewm.py look like potential bugs. any thoughts?