flake8-builtins
flake8-builtins copied to clipboard
A003 should be disabled by default
Consider the following example:
class User(Model):
id: str
def __str__(self):
return f"User(id:{self.id})"
user = User(id='123')
print(user.id)
Class attributes are almost never accessed by themselves without going through an object or a class, so what's the point of this rule? If you must preserve this rule, please disable it by default.
Thanks for using flake8-builtins and even spending the time to report the problems you have with it! (and sorry that you have problems with it 😓 )
I see the point, on data models indeed it makes more sense, in general classes though I still think that it should try to avoid using builtins... maybe #74 would be a better middle ground? 🤔
Would you agree on that direction? Would you have the extra time to actually do PR with the changes?
I don't think this issue has anything to do with #74. The easiest way to deal with this issue now is just to ignore A003 in .flake8
, which means any improvement has to start from disabling it by default so the users don't have to. There's few, if any reason for A003 to exist, you'll never shadow in any reasonable piece of Python code. I mean, can you do the following? Sure, but what possibly could be the reason to write this piece of code?
class C:
id = 1
print(id(C()))
The middle ground is to disable this rule by default, so if the one in a million person who needs it, he can turn it on.
Looking at the changelog seems that from version 1.3.0 we have this A003
error code, released +4 years ago (2018-04-13) and so far there were only 2 reports (yours is the third) and both of them were fine with either changing the code or adding A003
on the list of ignored codes.
I'm sorry if it's not your intention, but your way to tell what needs to be done rather than asking is a bit annoying to be honest.
You have multiple options:
- add
A003
on the list of ignored codes by flake8 - fork the project make the changes you want and have a local/public copy of it that follows your needs
- provide a patch upstream
- work towards the solution I was proposing (i.e. #74)
- refactor the code to not use the builtins
The idea of A003
when it was split from A001
was that, at least I personally, I'm interested in avoiding using builtin names anywhere, just for the tranquility of being able to copy a method from a class and making it a top level function without having to think if I need, then, to rename it.
just for the tranquility of being able to copy a method from a class and making it a top level function
A001 already takes care of this use case, either because of the assignment or the function name. You don't need A003. Perhaps you need an A004 to cover the case where you shadow a builtin on a function, but not methods of class variables, both of which places you can't access directly except in extraordinarily rare circumstances.
your way to tell what needs to be done rather than asking is a bit annoying to be honest
I don't have a question. I have a request, and I said please. Stating a position is common way to start a discussion, and I expect and will respect different but reasonable positions, I just fail to think of any. Would you like me to sandwich both of my messages with irrelevant greetings, appreciations and provide a list of rationales in multiple paragraphs? If that's the case perhaps you should state that clearly with a Github template, otherwise I'll act like how people have acted since the beginning of the internet on public forums and mailing lists.
provide a patch upstream
I take this as an agreement that A003 should be disabled by default.
Hi ! I'd like to second this suggestion.
In my project we had our own logging.Formatter
, e.g.
import logging
class MyFormatter(logging.Formatter):
def format(self, record):
...
which raises an A003
, clearly a false positive.
So I thought a bit about it and this warning does not make sense for classes. Classes are isolated scopes that don't leak symbols.
The purpose of warning about builtins is if the programmer declares something that overrides the builtins in globals()
or locals()
which is never the case inside a class.
So my suggestion is that this becomes a A903
warning instead. x9nn flake8 error codes typically mean opinionated warnings disabled by default.
Regardless, thank you so much for your work.
Hi, I'd also like to +1 this suggestion. This warning is a false positive because a class attribute can never conflict with the builtin.
For example, my custom LinkedList class is unable to use self.next
as an instance variable name seems strange to prohibit.
--builtins-ignorelist
doesn't fix my issue because I do want to block use of next
that actually shadow builtins
Hi, I think a better approach would be to refine the rule rather than disabling it.
Class attributes are only accessed by name during class definition (e.g. specifying argument defaults), which is a unique use-case.
Perhaps this rule can be split in two? One for general variables and one for variables inside classes?
@Rizhiy there already exists A001 and A004
@wyuenho Thanks, I guess then I agree that A003 is better disabled by default
I'm going to close this issue as the PR has been hanging around for over a year and the maintainer just chose to ignore it. I've switched over to ruff and I no longer use this package. Please open a new issue and discuss among yourselves. Hopefully the 11th person who is bothered by this problem is going to change his mind. Good luck.