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

[pylint] Implement singledispatch-method (E1519) #10140

Merged

Conversation

Glyphack
Copy link
Contributor

@Glyphack Glyphack commented Feb 26, 2024

Implementing the rule
https://pylint.readthedocs.io/en/stable/user_guide/messages/error/singledispatch-method.html#singledispatch-method-e1519

Implementation simply checks the function type and name of the decorators.

@Glyphack Glyphack force-pushed the pytlint/singledispatch-method-E1519 branch 2 times, most recently from bf7f97b to 3a5e666 Compare February 28, 2024 19:04
@Glyphack Glyphack changed the title Implement pylint E1519 singledispatch-method #970 linter: Implement pylint E1519 singledispatch-method #970 Feb 28, 2024
@Glyphack Glyphack marked this pull request as ready for review February 28, 2024 19:05
@Glyphack Glyphack force-pushed the pytlint/singledispatch-method-E1519 branch from 3a5e666 to 3111e95 Compare February 28, 2024 19:42
Copy link
Contributor

github-actions bot commented Feb 28, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+0 -3 violations, +0 -0 fixes in 1 projects; 42 projects unchanged)

pandas-dev/pandas (+0 -3 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

- pandas/_libs/tslibs/period.pyi:57:5: PLR0917 Too many positional arguments (9/5)
- pandas/_libs/tslibs/period.pyi:73:1: PLR0904 Too many public methods (24 > 20)
- pandas/_libs/tslibs/period.pyi:79:9: PLR0917 Too many positional arguments (10/5)

Changes by rule (2 rules affected)

code total + violation - violation + fix - fix
PLR0917 2 0 2 0 0
PLR0904 1 0 1 0 0

/// Checks for singledispatch decorators on class methods.
///
/// ## Why is this bad?
/// Single dispatch must happen on the type of first non self argument
Copy link
Contributor

Choose a reason for hiding this comment

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

Example code is helpful here. Take a look at other rules to see the format. The pylint docs contains examples to use.

You can also link the Python documentation of singledispatchmethod


@singledispatch # [singledispatch-method]
def move(self, position):
pass
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to catch @funcname.register as the pylint rule does?

Even if not, it would be good to include those examples in the tests in case we can do it in the future.

Copy link
Member

Choose a reason for hiding this comment

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

Probably a bit harder (but possible) to catch @funcname.register, but agree we should add tests for it with commentary.

@charliermarsh charliermarsh added rule Implementing or modifying a lint rule preview Related to preview mode features labels Feb 29, 2024
@charliermarsh charliermarsh changed the title linter: Implement pylint E1519 singledispatch-method #970 [pylint] Implement singledispatch-method (E1519) Mar 1, 2024
@charliermarsh
Copy link
Member

Gonna address feedback and merge.

@charliermarsh charliermarsh force-pushed the pytlint/singledispatch-method-E1519 branch from 1c52b00 to 1dbb6ae Compare March 1, 2024 02:02
@charliermarsh charliermarsh enabled auto-merge (squash) March 1, 2024 02:02
@charliermarsh charliermarsh force-pushed the pytlint/singledispatch-method-E1519 branch from 1dbb6ae to 6702c4d Compare March 1, 2024 02:04
@charliermarsh charliermarsh force-pushed the pytlint/singledispatch-method-E1519 branch from 3d75d73 to c814776 Compare March 1, 2024 02:15
@charliermarsh charliermarsh merged commit cbafae0 into astral-sh:main Mar 1, 2024
17 checks passed
@Glyphack Glyphack deleted the pytlint/singledispatch-method-E1519 branch March 1, 2024 08:50
@Glyphack
Copy link
Contributor Author

Glyphack commented Mar 1, 2024

Thanks for addressing the comments 😃

nkxxll pushed a commit to nkxxll/ruff that referenced this pull request Mar 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
preview Related to preview mode features rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants