Document expected or recommended annotations to avoid ANN101 and ANN102 errors
joseph-wakeling-frequenz opened this issue · 4 comments
Description
Currently flake8-annotations will report an error if the self or cls parameters are not annotated. It is not clear what flake8-annotations wants from the source code in order not to report these errors, and neither PEP 484 nor PEP 3107 offers any guidance as to how self or cls should be annotated. In fact PEP 484 suggests in Annotating class and instance methods that such annotation is not generally necessary:
In most cases the first argument of class and instance methods does not need to be annotated, and it is assumed to have the type of the containing class for instance methods, and a type object type corresponding to the containing class object for class methods.
This means that the out-of-the-box behaviour of flake8-annotations appears (at least at first glance) contrary to the guidelines of the PEPs it is supposed to be enforcing.
Given that neither PEP 484 nor PEP 3107 offer any general guidance on annotating self or cls parameters, the flake8-annotations README should add some examples of how to handle/avoid these errors.
Rationale/Use Case
Behaviour observed with flake8==3.7.9, flake8-annotations==2.0.1, running on CPython 3.7.6.
The error can be seen with even a very trivial class, e.g.:
class Foo:
def __call__(self) -> float:
return 1.0I'm assuming here that the answer is not something like:
class Foo:
def __call__(self: "Foo") -> float:
return 1.0which works, but is rather boilerplate-y and would make renaming a class very annoying.
I'm also assuming that the answer is not just "suppress that class of errors", e.g. with
flake8 --extend-ignore=ANN101,ANN102
which would seem a bit excessive given that there are probably a subset of cases (e.g. those mentioned in the PEP 484 link above) where one probably does want a self or cls annotation. (If it's possible to suppress ANN101 and 102 and still get validation of those special cases, it would be good to have a clear indication of that.)
Basically, please offer some clear guidance on this, given that the PEPs referred to would suggest that un-annotated self and cls is just fine most of the time, and hence flake8-annotations out-of-the-box behaviour is quite unintuitive.
This means that the out-of-the-box behaviour of
flake8-annotationsappears (at least at first glance) contrary to the guidelines of the PEPs it is supposed to be enforcing.
The purpose of this package is not to enforce the related PEPs, as they make no mandates regarding full annotation coverage. Its purpose is to detect the absence of these annotations; all errors are exposed and we leave it up to the developer to determine what subset of these errors to enforce for their project. This project, for example, currently ignores 6 of its own errors.
I'm assuming here that the answer is not something like:
Should you want to lint for these missing annotations that would indeed be one answer, in addition to the answers provided by PEP484. As an aside, note that with PEP 563 it's not necessary to use strings in 3.7+.
If it would be helpful we can include the links to these PEPs as footnotes in the README.
I'm also assuming that the answer is not just "suppress that class of errors"
That is the answer. If you don't want to enforce the presence of these annotations, then ignore them.
The purpose of this package is not to enforce the related PEPs, as they make no mandates regarding full annotation coverage. Its purpose is to detect the absence of these annotations; all errors are exposed and we leave it up to the developer to determine what subset of these errors to enforce for their project. This project, for example, currently ignores 6 of its own errors.
Fair enough. Thanks for the clarifications :-)
If it would be helpful we can include the links to these PEPs as footnotes in the README.
I'm not sure that's necessary, having the PEP numbers is enough to know how to look them up.
I'm also assuming that the answer is not just "suppress that class of errors"
That is the answer. If you don't want to enforce the presence of these annotations, then ignore them.
OK; thanks for the clarification. Would you consider amending the README with some advice or examples along these lines?
I ask because, as I said, it's not intuitive that a CI tool would object to normal/expected Python programming practice (of course it makes sense now I have your explanation). I don't think it would be reasonable to ask that these checks be ignored by default -- as you say, better to report all missing annotations and leave it to the user to decide what to do about that -- but it seems reasonable to offer some advice to users about recommended usage.
From my point of view it would suffice to say up-front pretty much exactly what you told me: the default behaviour is to report all missing annotations, but here are some common cases that you will probably want to ignore.
I'm happy to submit a PR with what I think would be good phrasing, if that would be welcome.
I've added a couple notes to #73, so we'll have them available once the new version is released.
Thanks very much for the feedback!
Thanks to you for the explanation and the README tweaks! :-)