-
Notifications
You must be signed in to change notification settings - Fork 206
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
N815: Ignore TypedDict class variable casing #189
base: main
Are you sure you want to change the base?
Conversation
Whilst this approach covers many scenarios, I can't see an easy way to handle mutliple layers of inheritance from |
@@ -448,6 +453,8 @@ class VariablesCheck(BaseASTCheck): | |||
def _find_errors(self, assignment_target, parents, ignore): | |||
for parent_func in reversed(parents): | |||
if isinstance(parent_func, ast.ClassDef): | |||
if "TypedDict" in parent_func.superclasses: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we skipping this altogether? People may in fact want to check this. I would instead think we should add an error code for this case so it can be ignored/disabled on a case-by-case basis
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question. Happy to have a go at implementing that. Would you go with something like N819: mixedCase variable in TypedDict
? Do you think we should disable by default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have pushed a new commit to implement this new error code, let me know what you think
We can't handle that at all. To do so would require running the code to get the |
I suspected as much but wasn't sure of what a solution might involve. Thanks |
3ec65fd
to
5ab9b69
Compare
5ab9b69
to
884f3c4
Compare
Prosed fix for #178
Add specific exemption from N815 for all subclasses of
TypedDict
because class variable naming conventions should not apply to dictionary keys.I've tried to reuse code from the rule for
Exceptions
(N818) as per the suggestion on #178.This is my first time working with the
ast
module or with anyflake8
plugin so please bear that in mind when reviewing this PR. Any advice on whether it's reasonable to tag everyClassDef
with the names of its superclasses, and potential performance concerns around that, would be very welcome. On balance, I thought it may be preferable to do this rather than determining the superclasses again for every variable in a class.Thanks :)