-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
TYP/CI: bump mypy&pyright #46905
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
TYP/CI: bump mypy&pyright #46905
Changes from 4 commits
11887ef
eea8fa7
b47075d
8e3669f
8f95666
6033597
b6f4a5c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -74,7 +74,7 @@ jobs: | |
|
||
- name: Install pyright | ||
# note: keep version in sync with .pre-commit-config.yaml | ||
run: npm install -g [email protected].245 | ||
run: npm install -g [email protected].246 | ||
|
||
- name: Build Pandas | ||
id: build | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -89,7 +89,7 @@ repos: | |
types: [python] | ||
stages: [manual] | ||
# note: keep version in sync with .github/workflows/code-checks.yml | ||
additional_dependencies: ['[email protected].245'] | ||
additional_dependencies: ['[email protected].246'] | ||
- repo: local | ||
hooks: | ||
- id: flake8-rst | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -957,7 +957,7 @@ def equals(self, other) -> bool: | |
return array_equivalent(left, right, dtype_equal=True) | ||
|
||
def _quantile( | ||
self: BaseMaskedArrayT, qs: npt.NDArray[np.float64], interpolation: str | ||
self, qs: npt.NDArray[np.float64], interpolation: str | ||
) -> BaseMaskedArray: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pyright didn't previously find this. I assume the return value can be any There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yep. maybe add type annotations to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can be follow-on. I suspect that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will leave this out - it would require quite a few ignore comments if we do something like: @overload
def _maybe_mask_result(self, result: ArrayLike, mask: ArrayLike) -> ArrayLike:
...
@overload
def _maybe_mask_result(
self, result: tuple[ArrayLike, ArrayLike], mask: ArrayLike
) -> tuple[ArrayLike, ArrayLike]:
...
def _maybe_mask_result(
self, result: ArrayLike | tuple[ArrayLike, ArrayLike], mask: ArrayLike
) -> ArrayLike | tuple[ArrayLike, ArrayLike]: For example |
||
""" | ||
Dispatch to quantile_with_mask, needed because we do not have | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mypy doesn't seem to understand
all(...)
to narrow the type.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this works, but let's not do this yet! (until we can use it elsewhere)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looking through the code there are a couple of other places this pattern is used that do not use
all_not_none
frompandas/core/common.py
maybe change this one here, then in the future we only need the TypeGuard in one place.
note need to prevent circular import.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have there been discussions about using
typing_extensions
? Then we could already use TypeGuard.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no we don't use typing_extensions but we can import from typing in pandas._typing with a guard so that it resolves to Any in older python versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As long as
typing_extensions
is not a run-time dependency, I would not be adverse to it being added as a dev dependency.