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

[pylint] Also reports case np.nan/case math.nan (PLW0177) #16378

Merged
merged 4 commits into from
Feb 26, 2025
Merged

[pylint] Also reports case np.nan/case math.nan (PLW0177) #16378

merged 4 commits into from
Feb 26, 2025
+55 −2

Conversation

InSyncWithFoo
Copy link
Contributor

Summary

Resolves #16374.

PLW0177 now also reports the pattern of a case branch if it is an attribute access whose qualified name is that of either np.nan or math.nan.

As the rule is in preview, the changes are not preview-gated.

Test Plan

cargo nextest run and cargo insta test.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Feb 25, 2025 •
edited
Loading

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@ntBre ntBre added the rule Implementing or modifying a lint rule label Feb 25, 2025
@MichaReiser
Copy link
Member

@ntBre could you take a look at this PR?

ntBre
ntBre approved these changes Feb 26, 2025
View reviewed changes
Copy link
Contributor

@ntBre ntBre left a comment

Choose a reason for hiding this comment

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

Thanks for your work on this and for the clarification on my match comments! I left a slightly different implementation strategy suggestion for your consideration, but I'm happy with this one too if you are.

ntBre
ntBre approved these changes Feb 26, 2025
View reviewed changes
Copy link
Contributor

@ntBre ntBre left a comment

Choose a reason for hiding this comment

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

Nice, thanks!

@ntBre ntBre merged commit 671494a into astral-sh:main Feb 26, 2025
21 checks passed
@InSyncWithFoo InSyncWithFoo deleted the PLW0177 branch February 26, 2025 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ntBre ntBre ntBre approved these changes

Assignees
No one assigned
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Do not match-case against np.nan
3 participants
@InSyncWithFoo @MichaReiser @ntBre