TYP: update for numpy-1.21.0 #42200
TYP: update for numpy-1.21.0 #42200
Conversation
| @@ -142,7 +137,7 @@ def arrays_to_mgr( | |||
|
|
|||
|
|
|||
| def rec_array_to_mgr( | |||
| data: MaskedRecords | np.recarray | np.ndarray, | |||
| data: np.ma.MaskedArray | np.recarray | np.ndarray, | |||
There was a problem hiding this comment.
There's a
# fill if needed
if isinstance(data, np.ma.MaskedArray):
new_arrays = fill_masked_arrays(data, arr_columns)
in the body that implies that MaskedArray (which is a superclass?) is allowed?
There was a problem hiding this comment.
In all tests that get here, we have isinstance(data, np.ma.mrecords.MaskedRecords). AFAICT the check is for MaskedArray bc MaskedRecords requires a separate import that we are avoiding at import-time
There was a problem hiding this comment.
MaskedRecords, MaskedArray and recarray are all subclass of np.ndarray so the annotation here is redundant and should just be np.ndarray
Since MaskedArray is a subclass of np.ndarray, it is a valid type to pass to rec_array_to_mgr and mypy is correct that we pass off MaskedArray classes and subclasses to fill_masked_arrays
In all tests that get here
Isn't the point of mypy to catch the cases where we don't have tests?
but ok to leave for now. as we now have a cast and comment to highlight the issue
pandas/core/nanops.py
Outdated
| @@ -1784,11 +1790,9 @@ def na_accum_func(values: ArrayLike, accum_func, *, skipna: bool) -> ArrayLike: | |||
| # DatetimeArray/TimedeltaArray | |||
| # TODO: have this case go through a DTA method? | |||
| # For DatetimeTZDtype, view result as M8[ns] | |||
| values = cast("DatetimeArray | TimedeltaArray", values) | |||
There was a problem hiding this comment.
i think just DatetimeArray (comment on L1790 should reflect this too); OK for follow-up.
There was a problem hiding this comment.
actually it's probably best to remove the cast and keep the "fix later" ignore. A 3rd party EA subclass could satisfy values.dtype.kind in ["m", "M"] and would not have a _simple_new method as that's not a requirement of the EA api.
|
The MaskedRecords change is the only one that looks questionable, everything else I think we can merge and cleanup in follow-ups. |
sure. will look at that in detail tomorrow. feel free to push changes here in the meantime. |
lithomas1
added
Typing
|
may need to do a 1.2.6 with this unfortunately |
|
@simonjayhawkins we need for 1.3 i assume. |
|
just pushed restoring the MaskedRecords annotation |
not strictly, but best not to diverge the ci before we release
There are no behavioral code changes that indicate that there are issues with 1.2.5 and numpy 1.21 |
|
we had testing on 1.2.x for numpy-dev, but if we do get reports of issues with 1.2.5 and numpy 1.21 we may well want to do a 1.2.6 to fix (and then backport these changes to 1.2.x also, 1.2.x is on mypy 0.782 but probably not an issue) |
…2205) Co-authored-by: Simon Hawkins <[email protected]>
Co-authored-by: Brock <[email protected]>
| @@ -107,6 +108,8 @@ | |||
| _int32_max = np.iinfo(np.int32).max | |||
| _int64_max = np.iinfo(np.int64).max | |||
|
|
|||
| NumpyArrayT = TypeVar("NumpyArrayT", bound=np.ndarray) | |||
There was a problem hiding this comment.
why can't we just use np.ndarray?
No description provided.