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.

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

Enable Ruff PLE (Pylint Error) #13305

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Enable Ruff PLE (Pylint Error) #13305

wants to merge 4 commits into from

Conversation

Avasam
Copy link
Collaborator

@Avasam Avasam commented Dec 26, 2024

Ref #13295
https://docs.astral.sh/ruff/rules/#error-e_1
This did found seemingly incorrect dunders in stubs that couldn't be caught by stubtest

@Avasam Avasam changed the title Enable Ruff PLE Enable Ruff PLE (Pylint Error) Dec 26, 2024

This comment has been minimized.

This comment has been minimized.

stubs/regex/regex/regex.pyi Outdated Show resolved Hide resolved
stubs/regex/regex/regex.pyi Outdated Show resolved Hide resolved
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@AlexWaygood
Copy link
Member

What do we do if we're writing stubs for a library that really does invalidly declare a special method? It's happened before. We'd be in a tough spot there: we'd either have to noqa the lint error (due to it ) or allowlist the stubtest error. It might be pretty confusing for a new contributor that they get a CI error whichever thing they do?

@Avasam
Copy link
Collaborator Author

Avasam commented Dec 28, 2024

What do we do if we're writing stubs for a library that really does invalidly declare a special method? [...] ]It might be pretty confusing for a new contributor that they get a CI error whichever thing they do?

We've had "unfixable" cases like that, I can namely remember around __all__ including names that don't exists. I personally think this is one of those rare cases where the best thing to do is to noqa it with a link to a report upstream.

I'd be very surprised, but if we add stubs for a library that does it on purpose, and often. Let's say overloads builtin dunders to add params and explicitly calls those methods instead of using keywords (del, in, repr, etc...). Then we could disable that rule for that stub (similarly to how we disable rules for proto generated stubs)

In other words, I think it's a rare/niche enough case and we have an escape hatch in case it does happen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants