pylint -- should I just ignore it sometimes?

S

Seebs

So, I'm messing around with pylint. Quite a lot of what it says
is quite reasonable, makes sense to me, and all that.

There's a few exceptions.

One: I am a big, big, fan of idiomatic short names where appropriate.
For instance:
catch <something>, e:
I don't want a long, verbose, name -- "e" is about as much in need of
a long and descriptive name as the stereotypical "i" one would use as
a loop index (in a language without iterators). Should I just ignore
that, or is it really more Pythonic to say something like:
catch KeyError, exception_resulting_from_the_use_of_a_key_not_defined_for_the_dictionary_in_which_it_was_looked_up:

Secondly: I am getting a couple of hits on things like "Too many instance
attributes (8/7) or "Too many branches (14/12)". In the cases in question,
it doesn't seem to me that the number of instance attributes is particularly
devastatingly complicated, because the instances in question are, by design,
sort of data stores; they carry a handful of precomputed results that are
then reused to populate templates.

So am I going to be laughed out of the room if I just let a class have
eight instance attributes, or use a short name for a caught exception?

-s
 
S

Steven D'Aprano

So, I'm messing around with pylint. Quite a lot of what it says is
quite reasonable, makes sense to me, and all that.

There's a few exceptions.

One: I am a big, big, fan of idiomatic short names where appropriate.
For instance:
catch <something>, e:

That would be except, not catch.

I don't want a long, verbose, name -- "e" is about as much in need of a
long and descriptive name as the stereotypical "i" one would use as a
loop index (in a language without iterators). Should I just ignore
that, or is it really more Pythonic to say something like:
catch KeyError,
exception_resulting_from_the_use_of_a_key_not_defined_for_the_dictionary_in_which_it_was_looked_up:


Well, that name is 98 characters, which means it's impossible to use it
without exceeding the 78 or 79 character per line limit, so I guess that
since there's no other alternative between 1 character and 98, you'll
just have to break down and ignore pylint.

So am I going to be laughed out of the room if I just let a class have
eight instance attributes, or use a short name for a caught exception?

Unfortunately, pylint is always right. Without exception. Like Microsoft
Word's grammar checker. As we all know, computers are much more capable
of capturing fine and subtle distinctions of meaning than we are, so we
should always follow their advice.

*wink*


Perhaps you should have a read of PEP 8, the recommended style guide.

http://www.python.org/dev/peps/pep-0008/

Take particular note of what it says about inconsistency.
 
A

Alexander Kapps

So, I'm messing around with pylint. Quite a lot of what it says
is quite reasonable, makes sense to me, and all that.

There's a few exceptions.

One: I am a big, big, fan of idiomatic short names where appropriate.
For instance:
catch<something>, e:
I don't want a long, verbose, name -- "e" is about as much in need of
a long and descriptive name as the stereotypical "i" one would use as
a loop index (in a language without iterators). Should I just ignore
that, or is it really more Pythonic to say something like:
catch KeyError, exception_resulting_from_the_use_of_a_key_not_defined_for_the_dictionary_in_which_it_was_looked_up:

catch KeyError, exc
catch KeyError, exception
Secondly: I am getting a couple of hits on things like "Too many instance
attributes (8/7) or "Too many branches (14/12)". In the cases in question,
it doesn't seem to me that the number of instance attributes is particularly
devastatingly complicated, because the instances in question are, by design,
sort of data stores; they carry a handful of precomputed results that are
then reused to populate templates.

So am I going to be laughed out of the room if I just let a class have
eight instance attributes, or use a short name for a caught exception?

-s

Pylint gives *hints* about *possible* problems or bad style. I
personally think it's good practice to take care of all of them and
carefully think if you or pylint is right. Once you decided that you
are right, you can configure pylint to not complain about certain
aspects. See man pylint for more.
 
S

Seebs

That would be except, not catch.

Er, yeah, that.
Well, that name is 98 characters, which means it's impossible to use it
without exceeding the 78 or 79 character per line limit, so I guess that
since there's no other alternative between 1 character and 98, you'll
just have to break down and ignore pylint.
:)

Perhaps you should have a read of PEP 8, the recommended style guide.

Will do.

-s
 
S

Shawn Milochik

Just to be pedantic (or maybe even helpful), the use of the comma
after the exception is deprecated in favor of 'as.'

So:

except ValueError as ex:

not:

except ValueError, ex:

I don't know how far back in Python versions this syntax reaches, but
if yours supports it then it's probably a good idea to get into the
habit.
This way, you can catch multiple exceptions in a tuple without
confusing the interpreter:

except ValueError, IndexError as ex:

Shawn
 
S

Seebs

Just to be pedantic (or maybe even helpful), the use of the comma
after the exception is deprecated in favor of 'as.'

Not in code that has to run on older Pythons.

I'm pretty sure I have to work with everything from 2.4 to 2.6 or so.

That reminds me, though. Speaking of deprecation, I have:
from string import Template
(see PEP 292 or so?), and pylint says "Uses of a deprecated module 'string'",
but I don't know of a way to get Template except by doing that.

-s
 
T

Terry Reedy

So, I'm messing around with pylint. Quite a lot of what it says
is quite reasonable, makes sense to me, and all that.

There's a few exceptions. ....
So am I going to be laughed out of the room if I just let a class have
eight instance attributes, or use a short name for a caught exception?

No, at least not be me.

Terry Jan Reedy
 
M

Martin P. Hellwig

So, I'm messing around with pylint. Quite a lot of what it says
is quite reasonable, makes sense to me, and all that.

There's a few exceptions.
Well, as with all styles IMHO, if there is a _good_ reason to break it,
then by all means do, but you might want to consider putting in a
comment why you did that and add the #pylint: disable-msg=<message_id>
on that line. If that is overkill, why not just comply to the standard
and avoid all the fuzz?
 
T

Terry Reedy

That reminds me, though. Speaking of deprecation, I have:
from string import Template
(see PEP 292 or so?), and pylint says "Uses of a deprecated module 'string'",
but I don't know of a way to get Template except by doing that.

A buggy PyLint is passing on bad (obsolete, deprecated ;-) info: In 3.1
['Formatter', 'Template', '_TemplateMetaclass', '__builtins__',
'__doc__', '__file__', '__name__', '__package__', '_multimap', '_re',
'ascii_letters', 'ascii_lowercase', 'ascii_uppercase', 'capwords',
'digits', 'hexdigits', 'maketrans', 'octdigits', 'printable',
'punctuation', 'whitespace']
 
S

Seebs

Well, as with all styles IMHO, if there is a _good_ reason to break it,
then by all means do, but you might want to consider putting in a
comment why you did that and add the #pylint: disable-msg=<message_id>
on that line. If that is overkill, why not just comply to the standard
and avoid all the fuzz?

Well, part of what I'm trying to understand is why the standard in question
says what it says. I'm pretty much mystified by a claim that something with
seven instance attributes is "too complicated". For instance, I've got a
class which represents (approximately) a C function, for use in writing
various wrappers related to it. It has name, return type, args, default
values, a list of arguments which need various modifications, a default
return value, and so on... And it ends up with, apparently, 10 instance
attributes.

I can't tell whether there's actually a general consensus that classes
should never be nearly that complicated, or whether pylint is being a little
dogmatic here -- I haven't seen enough other Python to be sure. I'm
used to having objects with anywhere from two or three to a dozen or more
attributes, depending on what kind of thing they model. It seems like a
very odd measure of complexity; is it really that unusual for objects to have
more than seven meaningful attributes?

-s
 
M

Martin P. Hellwig

On 10/19/10 23:36, Seebs wrote:
It seems like a
very odd measure of complexity; is it really that unusual for objects to have
more than seven meaningful attributes?
Speaking without context here, so take it with as much salt as required
;-), it is not that unusual. However there are some things to consider,
for example are all these attributes related to each other? If so
wouldn't it be more pythonic to have one attribute which is a dictionary
and store your stuff in there?

I follow pylint pretty strict and only tend to ignore it when making it
lint-friendly would mean making it less readable or less logically.

As everything pylint is a really useful tool especially when you just
start writing in python and it can give you valuable clues on how to
improve your programming. So just take it as hints that there might be
ways to write it better, have a think about it, perhaps ask as you have
done, and happily ignore it if all else fails :)
 
S

Seebs

Speaking without context here, so take it with as much salt as required
;-), it is not that unusual. However there are some things to consider,
for example are all these attributes related to each other? If so
wouldn't it be more pythonic to have one attribute which is a dictionary
and store your stuff in there?

I don't know. Does "pythonic" mean "needlessly verbose"? :)

I did, in an early draft, have something that basically came down to:

self.foo = {}
self.foo['a'] = a()
self.foo['b'] = b()

and then I realized that I could just write:
self.a = a()
self.b = b()

and spend a lot less extra typing on repeating something that, by virtue
of being repeated constantly, was therefore meaningless. It wasn't creating
a meaningful distinction, it wasn't showing a meaningful relationship...
All these things are attributes of the thing itself, not attributes of its
dictionary.
As everything pylint is a really useful tool especially when you just
start writing in python and it can give you valuable clues on how to
improve your programming. So just take it as hints that there might be
ways to write it better, have a think about it, perhaps ask as you have
done, and happily ignore it if all else fails :)

Part of the trick is I'm trying to figure out when the problem is that my
intuition about what makes code "readable" contradicts the norms of the python
community, and when the problem is just that pylint is being dogmatic where
real programmers would likely exercise judgement.

-s
 
S

Seebs

Tools like pylint are far more useful if every message they emit is
something that you must deal with, rather than ignore every time you see
it. That way, it's feasible to get the output to no messages at all, and
have a defensible reason for every disabled message.

That makes sense -- we do the same thing with warnings in C, usually.

I'll see whether I can find ways to, say, suppress a message for a specific
line rather than in general. (Though I have no idea what line number to
use for a warning about too many instance attributes...)

-s
 
A

Alexander Kapps

Well, part of what I'm trying to understand is why the standard in question
says what it says. I'm pretty much mystified by a claim that something with
seven instance attributes is "too complicated". For instance, I've got a
class which represents (approximately) a C function, for use in writing
various wrappers related to it. It has name, return type, args, default
values, a list of arguments which need various modifications, a default
return value, and so on... And it ends up with, apparently, 10 instance
attributes.

I can't tell whether there's actually a general consensus that classes
should never be nearly that complicated, or whether pylint is being a little
dogmatic here -- I haven't seen enough other Python to be sure. I'm
used to having objects with anywhere from two or three to a dozen or more
attributes, depending on what kind of thing they model. It seems like a
very odd measure of complexity; is it really that unusual for objects to have
more than seven meaningful attributes?

-s

The general idea is, that modules, classes, methods, and functions
should be small so they are better reusable, manageable and
understandable. If you have a huge class or function with many
attributes or local variables, it's often a sign, that your
class/function does to much and you better refactor this into
smaller pieces. There isn't and there can't be a general consensus
about /how/ small some part should be.

If pylint complains about too many variables or such, take it as a
hint to rethink your design. If you say, my design is good, then
feel free to ignore the warning.

If your classes wrap some existing datastructure and pyling
complains, take it as a hint (just a hint, not more) that maybe your
underlying datastructure is to complex.

But there are no general rules. In the end you (the programmer) has
to decide how the code or the data is structured, pylint can only
give you hints, that there *may* be a problem.

I don't know why "the standard" (what standard?) says what it says,
but I guess, it's the result of long time experiences and analysis
of existing code. Trust them, unless you are sure to know better.
 
S

Seebs

The general idea is, that modules, classes, methods, and functions
should be small so they are better reusable, manageable and
understandable.

Makes sense.
If you have a huge class or function with many
attributes or local variables, it's often a sign, that your
class/function does to much and you better refactor this into
smaller pieces.

That is often practical, but it can involve tradeoffs -- there can be
a great deal of additional complexity from the interactions between the
refactored pieces.

I tend to refactor mostly if the pieces have some kind of value outside
their original context. In some cases, a process is just, well, sorta long,
but there's no relevance to breaking the components out -- they're not
separately useful anywhere.
I don't know why "the standard" (what standard?) says what it says,
but I guess, it's the result of long time experiences and analysis
of existing code. Trust them, unless you are sure to know better.

Well, one of the reasons I'm asking is to find out, from experienced
developers, how trustworthy the pylint people are, and what the target
audience is... Should I be treating this the way I'd treat a list of
coding style rules handed down by a first year CS teacher, which are great
rules for first year CS students, or the way I'd be treating a corporate
style document which is a mandated part of continued employment?

-s
 
S

Seebs

It's a code smell. Many discrete attributes is a sign that the design
can be improved by combining related attributes into a complex type.

Ahh.

I think that makes sense. In this case, I don't think it's worth it,
but I can see why it would be in some cases. There are cases where there's
a thing that really DOES have that many attributes, but I've seen lots
of cases where it made sense to subdivide them.

-s
 
J

Jean-Michel Pichavant

Seebs said:
Speaking without context here, so take it with as much salt as required
;-), it is not that unusual. However there are some things to consider,
for example are all these attributes related to each other? If so
wouldn't it be more pythonic to have one attribute which is a dictionary
and store your stuff in there?

I don't know. Does "pythonic" mean "needlessly verbose"? :)

I did, in an early draft, have something that basically came down to:

self.foo = {}
self.foo['a'] = a()
self.foo['b'] = b()

and then I realized that I could just write:
self.a = a()
self.b = b()

and spend a lot less extra typing on repeating something that, by virtue
of being repeated constantly, was therefore meaningless. It wasn't creating
a meaningful distinction, it wasn't showing a meaningful relationship...
All these things are attributes of the thing itself, not attributes of its
dictionary.

Difficult to argue about meaning when you give a meaningless example :)
The question you have to answer is : "Is a and b attributes of self
(whatever that is)".

Actually everything should be driven by its meaning, not if it's quicker
to write or something like that.

Regarding the first question about ignoring pylint:
It's a tool, and requires a lot of configuration.
1/ define *your* coding rules (PEP 8 is just one coding rule among the
others, it's only required if you want to commit into the standard library)
2/ When you know about your rules then configure pylint so it suits with
those rules.
3/ Never ignore a defect reported by pylint. If a pylint rule does not
satisfy you, disable it so pylint stops reporting it.



except ValueError, e:

Use meaningful names, this is so important. 'e' is not meaningful.
'exception' would be slighly better. On that particular case, pylint is
right to complain. When dealing with such issue like "I'm too lazy to
write proper engligh" always think that what is the most important thing
is to save the Reader's time, not the Writer's time. Using 'e' would
force to reader to introspect to code to get an idea of what 'e' is.
Moreover, code completion makes use of long names costless.

You should read this very good paper about naming:
http://tottinge.blogsome.com/meaningfulnames/

JM
 
S

Seebs

except ValueError, e:
Use meaningful names, this is so important.

It's important, but meaning can come from idiom, not from the word used.
'e' is not meaningful.

Sure it is. It's like i for a loop index.

There's a reason mathematicians use x, not the_value_that_can_vary.
When dealing with such issue like "I'm too lazy to
write proper engligh" always think that what is the most important thing
is to save the Reader's time, not the Writer's time. Using 'e' would
force to reader to introspect to code to get an idea of what 'e' is.

If the reader can't track a single-letter idiomatic name over two consecutive
lines of code, I don't think I can save the reader time. The reader is beyond
my ability to help at that point.

Furthermore, long names take longer to read and process.

As a *reader*, I vastly prefer:
except foo, e:
print "Fatal error:", e
to anything you could do with a longer name.

Consider, admittedly a C example:
for (int i = 0; i < n; ++i)
a += 1;
vs:
for (int index_into_array = 0; index_into_array < size_of_array; ++index_into_array)
array_into_which_we_are_indexing[index_into_array] += 1;

Here's the thing. The issue is not that I'm too lazy to write English. The
issue is that English, like every other language, USES PRONOUNS. Because
pronouns are a good way to MASSIVELY streamline communication. It's not just
a convenience to the writer/speaker; it also makes life easier for the
reader/listener.

Short variable names are pronouns. They make sense when you'd use a
pronoun in English.

"If we catch a KeyError, we print an error message including it."

We'd use a pronoun. Makes sense to use a short variable name.
Moreover, code completion makes use of long names costless.

No, it doesn't. It preserves their higher cognitive load in parsing.

Dogmatism about rejecting short variable names is inconsistent with the
psychology of human readers. Compilers don't care about length of
variable names. Humans do, and there are times when they benefit more
from a short and recognizeable name than they do from a long and
"expressive" one.

-s
 
S

Steven D'Aprano

except ValueError, e:

Use meaningful names, this is so important. 'e' is not meaningful.
'exception' would be slighly better.

While I agree with everything else you had to say, I have to take
exception to this comment [pun intended].

"e" as a short name for a generic exception instance is perfectly
reasonable, like:

i, j, k for an index, or a loop variable
e.g. for i in range(100)
n for some other integer variable
s for a string
x for a float, or an arbitrary sequence object
e.g. [x.spam() for x in some_sequence]

and similar.

The last example is very instructive. What do you gain by racking your
brain for a "more meaningful" name instead of x? The obvious
alternatives, "obj" or "item", are equally generic as "x", they don't add
any further information. And how much information do you need? It's easy
to parody:

[some_sequence_item.spam() for some_sequence_item in some_sequence]

The very shortness of the name is valuable because it reduces the *human*
parsing time in reading, and there is no cost because the conventions are
so familiar. The convention of "for i in ..." says "this is a loop over
an integer" so strongly, that I would argue that "for index in ..." would
actually *delay* comprehension.

Furthermore, the use of a single letter cues the reader that this
variable isn't notable -- there's nothing unusual or unconventional about
it, or it isn't the important part of the algorithm, or that its scope is
severely limited. For instance, consider the classic example of
exchanging two variables in Python:

a, b = b, a

versus:

thing, other_thing = other_thing, thing

The first example puts the emphasis on the *technique*, not the
variables. The second obscures it behind needlessly longer but still
generic names.

You are absolutely right to insist on meaningful variable names. Where
you go wrong is to assume that single letter names can't be meaningful.
 
M

Matteo Landi

Another situation in which I needed to disable such kind of warnings
is while working with graphics modules.
I often use variable names such as x, y, z for coordinates, or r,g,b for colors.
Would longer names make the reader's life easier?

Best regards,
Matteo

except ValueError, e:

Use meaningful names, this is so important. 'e' is not meaningful.
'exception' would be slighly better.

While I agree with everything else you had to say, I have to take
exception to this comment [pun intended].

"e" as a short name for a generic exception instance is perfectly
reasonable, like:

i, j, k for an index, or a loop variable
   e.g. for i in range(100)
n for some other integer variable
s for a string
x for a float, or an arbitrary sequence object
   e.g. [x.spam() for x in some_sequence]

and similar.

The last example is very instructive. What do you gain by racking your
brain for a "more meaningful" name instead of x? The obvious
alternatives, "obj" or "item", are equally generic as "x", they don't add
any further information. And how much information do you need? It's easy
to parody:

[some_sequence_item.spam() for some_sequence_item in some_sequence]

The very shortness of the name is valuable because it reduces the *human*
parsing time in reading, and there is no cost because the conventions are
so familiar. The convention of "for i in ..." says "this is a loop over
an integer" so strongly, that I would argue that "for index in ..." would
actually *delay* comprehension.

Furthermore, the use of a single letter cues the reader that this
variable isn't notable -- there's nothing unusual or unconventional about
it, or it isn't the important part of the algorithm, or that its scope is
severely limited. For instance, consider the classic example of
exchanging two variables in Python:

a, b = b, a

versus:

thing, other_thing = other_thing, thing

The first example puts the emphasis on the *technique*, not the
variables. The second obscures it behind needlessly longer but still
generic names.

You are absolutely right to insist on meaningful variable names. Where
you go wrong is to assume that single letter names can't be meaningful.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,769
Messages
2,569,582
Members
45,067
Latest member
HunterTere

Latest Threads

Top