Skip to content

Conversation

@ThiefMaster
Copy link
Contributor

Summary

This brings ruff's behavior in line with what pep8-naming already does and thus closes #8397.

I had initially implemented this to look at the last segment of a dotted path only when the entry in the *-decorators setting started with a ., but in the end I thought it's better to remain consistent w/ pep8-naming and doing a match against the last segment of the decorator name in any case.

If you prefer to diverge from this in favor of less ambiguity in the configuration let me know and I'll change it so you would need to put e.g. .expression in the classmethod-decorators list.

Test Plan

Tested against the file in the issue linked below, plus the new testcase added in this PR.

@ThiefMaster ThiefMaster force-pushed the n80x-decorators-whitelist branch from 335c989 to bc25465 Compare November 9, 2023 23:28
@ThiefMaster ThiefMaster force-pushed the n80x-decorators-whitelist branch from bc25465 to c80e0b2 Compare November 9, 2023 23:40
@github-actions
Copy link
Contributor

github-actions bot commented Nov 9, 2023

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@charliermarsh
Copy link
Member

I went back and forth on this a bit but ultimately I think it makes sense in the interest of pragmatism and matching user expectation.

@charliermarsh charliermarsh enabled auto-merge (squash) November 10, 2023 02:00
Copy link
Member

@charliermarsh charliermarsh left a comment

Choose a reason for hiding this comment

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

Thanks @ThiefMaster! I just modified the PR to break the logic out into standalone functions, and we also now always run the match checks rather than only if it's not an import.

@charliermarsh charliermarsh added the configuration Related to settings and configuration label Nov 10, 2023
@charliermarsh charliermarsh changed the title Fix {class,static}method-decorators for dotted names Support local and dynamic class- and static-method decorators Nov 10, 2023
@charliermarsh charliermarsh enabled auto-merge (squash) November 10, 2023 02:01
@charliermarsh charliermarsh merged commit 4ebd0bd into astral-sh:main Nov 10, 2023
@ThiefMaster ThiefMaster deleted the n80x-decorators-whitelist branch November 10, 2023 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

configuration Related to settings and configuration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

N805 not compatible with sqlalchemy hybrids

2 participants