Pylint false positives
Jon Ribbens
jon+usenet at unequivocal.eu
Wed Aug 15 06:44:57 EDT 2018
On 2018-08-15, Steven D'Aprano <steve+comp.lang.python at pearwood.info> wrote:
> On Tue, 14 Aug 2018 15:18:13 +0000, Jon Ribbens wrote:
>> On 2018-08-14, Steven D'Aprano <steve+comp.lang.python at pearwood.info>
>> wrote:
>>> # === process abstract methods en masse ===
>>> for name in "method_a method_b method_c method_d".split():
>>> @abstractmethod
>>> def inner(self):
>>> raise NotImplementedError
>>> inner.__name__ = name
>>> # This is okay, writing to locals works inside the class body.
>>> locals()[name] = inner
>>>
>>> del inner, name # Clean up the class namespace.
>>
>> You have a peculiar idea of "good style"...
>
> Yes, very peculiar. It's called "factor out common operations" and "Don't
> Repeat Yourself" :-)
>
> In a world full of people who write:
>
> d[1] = None
> d[2] = None
> d[3] = None
> d[4] = None
>
> I prefer to write:
>
> for i in range(1, 5):
> d[i] = None
>
> Shocking, I know.
Obviously what I'm objecting to is not the presence of a loop,
it's the presence of many obscure, bodgey and/or broken features
all lumped together:
* code running directly under the class definition
* creating a method then changing its name with foo.__name__
* poking things into to the class namespace with locals()
* using @abstractmethod without metaclass=ABCMeta
* dynamically adding @abstractmethod methods to a class
(Not to mention your code means the methods cannot have meaningful
docstrings.)
I would refuse a pull request containing code such as the above,
unless the number of methods being dynamically created was much
larger than 4, in which case I would refuse it because the design
of a class requiring huge numbers of dynamically created methods is
almost certainly fundamentally broken.
>> No - if you think about it, there's no way Pylint could possibly know
>> that the above class has methods method_a, method_b, etc.
>
> Well, if a human reader can do it, a sufficiently advanced source-code
> analyser could do it too... *wink*
If we get there, we won't need to do computer programming since we'll
have strong AI and we'll just say "computer, obey my wishes" :-)
> Yes, of course you are right, in practical terms I think it is extremely
> unlikely that PyLint or any other linter is smart enough to recognise
> that locals()[name] = inner is equivalent to setting attributes method_a
> etc. I actually knew that... "although to be honest I'm not sure" is an
> understated way of saying "It isn't" :-)
It's not so much the locals()[name] thing as much as that Pylint would
have to know what the computed values of `name` would be.
>> It also doesn't like the `del inner, name` because theoretically
>> neither of those names might be defined, if the loop executed zero
>> times.
>
> That's a limitation of the linter. Don't blame me if it is too stupid to
> recognise that looping over a non-empty string literal cannot possibly
> loop zero times :-)
Well, it's not looping over a string literal, it's looping over the
output of a method call on a string literal. You could *maybe* argue
that "literal words here".split() is sufficiently idiomatic that
Pylint should understand it, and the Pylint issues page would be an
excellent place to do so ;-)
More information about the Python-list
mailing list