Skip to content
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.

Sign up for GitHub

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

Jump to bottom

TYP: update mypy and small pyi fixes from ruff #54085

Merged
merged 6 commits into from
Jul 13, 2023
Merged

TYP: update mypy and small pyi fixes from ruff #54085

merged 6 commits into from
Jul 13, 2023

Conversation

twoertwein
Copy link
Member

There is a much newer version of pyright but I haven't yet managed to get it to pass.

twoertwein
twoertwein commented Jul 11, 2023
View reviewed changes
pandas/core/series.py
@@ -3459,7 +3458,7 @@ def sort_values(
self,
*,
axis: Axis = ...,
ascending: bool | int | Sequence[bool] | Sequence[int] = ...,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is undocumented (and incompatible with the declarations in NDFrame) - but it might work at runtime.

twoertwein
twoertwein commented Jul 11, 2023
View reviewed changes
pandas/core/series.py
@@ -3482,11 +3481,25 @@ def sort_values(
) -> None:
...

@overload
def sort_values(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NDFrame declared this fallback overload, mypy seems to be stricter now and requires it also in the subclasses.

@mroeschke mroeschke added the Typing type annotations, mypy/pyright type checking label Jul 11, 2023
@twoertwein twoertwein mentioned this pull request Jul 11, 2023
Closed
Dr-Irv
Dr-Irv approved these changes Jul 12, 2023
View reviewed changes
Copy link
Contributor

@Dr-Irv Dr-Irv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. Will let others approve/merge.

@twoertwein
Copy link
Member Author

This looks good to me. Will let others approve/merge.

Would still need to decide whether we prefer to disable the ruff/pylint rule or add ignore comments for pyright. Currently, I disabled the rule for ruff but not yet pylint.

@mroeschke
Copy link
Member

I would slightly prefer enabling the pylint/ruff rules and adding pyright ignores, if there are not too many ignores needed

@twoertwein
Copy link
Member Author

I would slightly prefer enabling the pylint/ruff rules and adding pyright ignores, if there are not too many ignores needed

Using the pyright comments now

@mroeschke mroeschke added this to the 2.1 milestone Jul 13, 2023
@mroeschke mroeschke merged commit 9ff3322 into pandas-dev:main Jul 13, 2023
@mroeschke
Copy link
Member

Thanks @twoertwein

@twoertwein twoertwein deleted the ruff_mypy branch August 9, 2023 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Dr-Irv Dr-Irv Dr-Irv approved these changes

@mroeschke mroeschke mroeschke approved these changes

@WillAyd WillAyd Awaiting requested review from WillAyd WillAyd is a code owner

Assignees
No one assigned
Labels
Typing type annotations, mypy/pyright type checking
Projects
None yet
Milestone
2.1
Development

Successfully merging this pull request may close these issues.
3 participants
@twoertwein @mroeschke @Dr-Irv