Discussion:
[Python-projects] pylint: False positive about field initialisation
Maarten ter Huurne
2006-03-28 23:06:41 UTC
Permalink
Hi,

On the following program:
===
class C:
def __init__(self):
self.set(self, 'abc')
def set(self, value):
self.__value = value
self.__length = len(value)
===
pylint reports:
===
W0201: 5:C.set: Attribute '__value' defined outside __init__
W0201: 6:C.set: Attribute '__length' defined outside __init__
===

Although strictly speaking they are indeed defined outside __init__, these
fields are guaranteed to be initialised when an object of type C is
constructed. It would be useful if pylint could recognise situations like
this one and not issue this warning.

Bye,
Maarten
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.logilab.org/pipermail/python-projects/attachments/20060328/d2cdd9bf/attachment.htm
Sylvain Thénault
2006-04-19 00:10:27 UTC
Permalink
Post by Maarten ter Huurne
Hi,
===
self.set(self, 'abc')
self.__value = value
self.__length = len(value)
===
===
W0201: 5:C.set: Attribute '__value' defined outside __init__
W0201: 6:C.set: Attribute '__length' defined outside __init__
===
Although strictly speaking they are indeed defined outside __init__, these
fields are guaranteed to be initialised when an object of type C is
constructed. It would be useful if pylint could recognise situations like
this one and not issue this warning.
hum, I'm not sure about this one... I find it easier when you
read code to have only one place to look at for which attributes are
defined on a class (I know that for instance inheritance isn't
necessarily on that way, and I don't want to repeat inherited attributes
in derived class initializer but...) than to have to dig into each
method that call the initializer. If I've to use such "reset" methods, I
usually begin by defining attributes to None in the initializer and then
call the "reset" methods, however I'm not sure I would do this if pylint
wasn't warn me about this (probably not actually).

Does other people here on the list have an opinion about this ?
--
Sylvain Th?nault LOGILAB, Paris (France).

http://www.logilab.com http://www.logilab.fr http://www.logilab.org
skip at pobox.com ()
2006-04-19 00:20:51 UTC
Permalink
Post by Maarten ter Huurne
===
W0201: 5:C.set: Attribute '__value' defined outside __init__
W0201: 6:C.set: Attribute '__length' defined outside __init__
Sylvain> hum, I'm not sure about this one... I find it easier when you
Sylvain> read code to have only one place to look at for which
Sylvain> attributes are defined on a class ...

On a related note...

A lot of the code in the project I work on looks something like this:

class Foo:
def __init__(self, ...):
self.attr = None

def start(self):
self.attr = SomethingElse(...)

def get_value(self, value):
return self.attr.get(value)

That is, attr can't be properly initialized at instantiation, but only after
have some other external setup has been performed (which is complete when
the start method is called). Initializing to None in __init__ is our
typical way of saying, "this attribute has not really been initialized yet".
Unfortunately, pylint assumes self.attr always has a value of None and
complains about the reference to self.attr.get in the get_value method.

I'm sure there must be some suppression possible, but I'd like it do
recognize that self.attr is assigned to more than once and that it can't
assume its value is None.

Skip
Sylvain Thénault
2006-04-19 00:24:13 UTC
Permalink
Post by skip at pobox.com ()
Post by Maarten ter Huurne
===
W0201: 5:C.set: Attribute '__value' defined outside __init__
W0201: 6:C.set: Attribute '__length' defined outside __init__
Sylvain> hum, I'm not sure about this one... I find it easier when you
Sylvain> read code to have only one place to look at for which
Sylvain> attributes are defined on a class ...
On a related note...
self.attr = None
self.attr = SomethingElse(...)
return self.attr.get(value)
That is, attr can't be properly initialized at instantiation, but only after
have some other external setup has been performed (which is complete when
the start method is called). Initializing to None in __init__ is our
typical way of saying, "this attribute has not really been initialized yet".
Unfortunately, pylint assumes self.attr always has a value of None and
complains about the reference to self.attr.get in the get_value method.
You mean that you get something like 'None has no "get" attribut' (eg
E1101) when pylint is analyzing the .get_value method ?
Post by skip at pobox.com ()
I'm sure there must be some suppression possible, but I'd like it do
recognize that self.attr is assigned to more than once and that it can't
assume its value is None.
hum, that's what it should do. I'll take a look at this tomorrow.

Cheers
--
Sylvain Th?nault LOGILAB, Paris (France).

http://www.logilab.com http://www.logilab.fr http://www.logilab.org
skip at pobox.com ()
2006-04-19 00:40:54 UTC
Permalink
Post by skip at pobox.com ()
Unfortunately, pylint assumes self.attr always has a value of None
and complains about the reference to self.attr.get in the get_value
method.
Sylvain> You mean that you get something like 'None has no "get"
Sylvain> attribut' (eg E1101) when pylint is analyzing the .get_value
Sylvain> method ?

Precisely.
Post by skip at pobox.com ()
I'm sure there must be some suppression possible, but I'd like it do
recognize that self.attr is assigned to more than once and that it
can't assume its value is None.
Sylvain> hum, that's what it should do. I'll take a look at this tomorrow.

Much appreciated.

Skip
Sylvain Thénault
2006-04-19 16:50:53 UTC
Permalink
Post by skip at pobox.com ()
Post by skip at pobox.com ()
Unfortunately, pylint assumes self.attr always has a value of None
and complains about the reference to self.attr.get in the get_value
method.
Sylvain> You mean that you get something like 'None has no "get"
Sylvain> attribut' (eg E1101) when pylint is analyzing the .get_value
Sylvain> method ?
Precisely.
Post by skip at pobox.com ()
I'm sure there must be some suppression possible, but I'd like it do
recognize that self.attr is assigned to more than once and that it
can't assume its value is None.
Sylvain> hum, that's what it should do. I'll take a look at this tomorrow.
I'm not able to reproduce this:

***@crater:pylint$ cat ~/testskipm.py
from UserDict import UserDict

class Foo:

def __init__(self):
self.attr = None

def start(self):
self.attr = UserDict()

def get_value(self, value):
return self.attr.get(value)
***@crater:pylint$ PYTHONPATH= pylint --version
No config file found, using default configuration
pylint 0.10.0,
astng 0.15.1, common 0.15.0
Python 2.3.5 (#2, Mar 8 2006, 06:37:07)
[GCC 4.0.3 20060304 (prerelease) (Debian 4.0.2-10)]
***@crater:pylint$
***@crater:pylint$
***@crater:pylint$ pylint -rn ~/testskipm.py
No config file found, using default configuration
************* Module testskipm
C: 0: Missing docstring
C: 0: Missing required attribute "__revision__"
C: 3:Foo: Missing docstring
C: 8:Foo.start: Missing docstring
C: 11:Foo.get_value: Missing docstring


Are you sure you're using the latest pylint/astng versions ?
--
Sylvain Th?nault LOGILAB, Paris (France).

http://www.logilab.com http://www.logilab.fr http://www.logilab.org
skip at pobox.com ()
2006-04-19 18:32:00 UTC
Permalink
Sylvain> I'm not able to reproduce this:
...
Sylvain> Are you sure you're using the latest pylint/astng versions ?

I think so:

% pylint --version
pylint 0.10.0,
astng 0.15.0, common 0.15.0
Python 2.4.2 (#1, Feb 23 2006, 12:48:31)
[GCC 3.4.1]

If I get a few minutes I'll see if I can come up with a self-contained
example.

Skip
Sylvain Thénault
2006-04-19 18:36:30 UTC
Permalink
Post by skip at pobox.com ()
...
Sylvain> Are you sure you're using the latest pylint/astng versions ?
% pylint --version
pylint 0.10.0,
astng 0.15.0, common 0.15.0
^^^^^^

a 0.15.1 release is available for astng
--
Sylvain Th?nault LOGILAB, Paris (France).

http://www.logilab.com http://www.logilab.fr http://www.logilab.org
skip at pobox.com ()
2006-04-19 00:24:19 UTC
Permalink
Sylvain> If I've to use such "reset" methods, I usually begin by
Sylvain> defining attributes to None in the initializer and then call
Sylvain> the "reset" methods, however I'm not sure I would do this if
Sylvain> pylint wasn't warn me about this (probably not actually).

Same here. I find it violates the DRY principle (don't repeat yourself) to
have to execute:

self.foo = self.bar = None
self.reset()

then have the reset method do the real initialization. Plus there's that
problem that pylint seems to think the attribute always has a value of None.

Skip
Loading...