TYP: Subset of "Improved the type stubs in the _libs directory to help with type checking" #44251
Conversation
|
Hello @twoertwein! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2021-12-12 19:24:28 UTC |
| # Union[Union[str, int, float, bool] Union[Period, Timestamp, Timedelta, Any]]]", | ||
| # expected "Tuple[NumpyArrayT, Union[Union[str, int, float, bool], Union[Period, | ||
| # Timestamp, Timedelta, Any]]]") | ||
| return upcast_values, fill_value # type: ignore[return-value] |
There was a problem hiding this comment.
Should probably be np.ndarray and not be NumpyArrayT since the dtype might change.
There was a problem hiding this comment.
The dtype can change but the Python type (i.e. np.ndarray or subclass) maybe preserved. I'm not sure but the numpy types don't annotate as preserving Python object type so maybe not. This was added for ma.MaskedArray handling in the constructors, so could well be that the numpy types are not precise enough, creating this false positive.
There was a problem hiding this comment.
looks ok to me. can you rebase @twoertwein
@simonjayhawkins @jbrockmendel if comments
|
cc @Dr-Irv |
Some tests are typed checked, including those files.
The test changes I made are technically not needed for this PR. They will be needed for |
IMHO, I think the changes to the test files should be left out for now. That will make the PR smaller. But I would defer to @simonjayhawkins on this |
pandas/core/ops/mask_ops.py
Outdated
| if left_mask is None: | ||
| return kleene_xor(right, left, right_mask, left_mask) | ||
|
|
||
| assert isinstance(left, np.ndarray) |
There was a problem hiding this comment.
Is this safe? @jbrockmendel The other kleene functions have it as well.
There was a problem hiding this comment.
holds in all of our current usages; to be future-safe would need to change this to a check that raises TypeError
There was a problem hiding this comment.
it raises now a TypeError
|
looks good, can you rebase @simonjayhawkins if any comments. |
| # Union[Union[str, int, float, bool] Union[Period, Timestamp, Timedelta, Any]]]", | ||
| # expected "Tuple[NumpyArrayT, Union[Union[str, int, float, bool], Union[Period, | ||
| # Timestamp, Timedelta, Any]]]") | ||
| return upcast_values, fill_value # type: ignore[return-value] |
There was a problem hiding this comment.
The dtype can change but the Python type (i.e. np.ndarray or subclass) maybe preserved. I'm not sure but the numpy types don't annotate as preserving Python object type so maybe not. This was added for ma.MaskedArray handling in the constructors, so could well be that the numpy types are not precise enough, creating this false positive.
The test modules are type checked but the test functions are not (check_untyped_defs is False). Saying that we have several test modules ignored in the mypy config |
|
@twoertwein can you merge master |
|
thanks @twoertwein very nice |
The two big parts missing from #43744 are interval.pyi and offsets.pyi as they result in many mypy errors.
I had already made changes to somewhat accommodate for interval.pyi and offsets.pyi (if these files are included, we have now "only" ~100 mypy errors). I kept these changes in this PR.