Mailman 3 September 2013 - code-quality

__len__ but no __getitem__
by Skip Montanaro 23 Sep '13

23 Sep '13

Having demonstrated my confusion about early breaks from loops, I will proceed to demonstrate my confusion about containers. I have a queue-like class in which I implement __len__ but not __getitem__. Pylint complains: timeddata.py:79: [R0924(incomplete-protocol), TimedDataQueue] Badly implemented Container, implements __len__ but not __getitem__ I can see where that would be a valid complaint for an array-like container, but not all containers should support indexing even if they have a measurable length. Python sets are one example: >>> s = set(range(10)) >>> len(s) 10 >>> s[0] Traceback (most recent call last): File "<stdin>", line 1, in <module> TypeError: 'set' object does not support indexing Queue-like containers seem similar. While I would allow access to the element at one end or the other (depending if I want queue-like or stack-like behavior), I think it would violate the definition of those types to allow indexing. Should pylint really be this strict? Or am I expected to implement everything necessary for an array-like containiner and just raise exceptions in those methods the user really shouldn't access? Thanks, Skip

3 4

Pylint and multiprocessing
by Johan Dahlin 19 Sep '13

19 Sep '13

Hi, I'm Johan and I'm working on a python based ERP for the brazilian market[1], it's written in Python. As part of our software development process we run pyflakes/pep8/pylint/unittests/coverage validation _before_ each commit can be integrated into the mainline. The main bottleneck of this validation turns out to be pylint and I started to investigate on how to use multiprocessing to speed that up. Attached is a first take on just that. This creates a configurable number of processes and uses them to split out the work on a per filename basis. I've been working on 0.26.0 of Pylint, which is the version included with Ubuntu 13.04. It seems that the part modified by this patch hasn't changed significantly in trunk, but I can port over my work to a newer version if there's interest. The fact that the different processes are not sharing state between each other probably may mean that certain checks are not going to work 100%, I don't fully understand how pylint works internally, perhaps someone else could chip in. Anyway, some numbers: i3-3220 (1 socket, 2 cores, 4 hyperthreads) Sample set 1: (37kloc, 92 files) - unmodified 18.35s 0% - n_procs = 1 18.50s -1% - n_procs = 2 11.29s +38% - n_procs = 3 10.96s +40% - n_procs = 4 10.85s +41% Sample set 2: (156kloc, 762 files) - unmodified 77.62s - n_procs = 1 82.33s -6% - n_procs = 2 47.68s +39% - n_procs = 3 46.04s +41% - n_procs = 4 45.19s +42% Xeon(R) CPU E5410 (2 sockets, 8 cores, no hyperthreading) Sample set 2: (156kloc, 762 files) - unmodified 140.996s - n_procs = 4 48.675s 65% - n_procs = 8 36.323s 74% The number seems fairly promising and introducing some simple errors in the code base are properly being caught, I didn't quite test any errors that require deep knowledge of other modules. The Xeon CPU is pretty old and thus each core is quite a bit slower than the i3. Running with n_processes = 1 seems to be a little bit slower, especially when there's a lot of source code, I suspect that it's due to the fact that the multiprocessing module is imported and perhaps the overhead of creating a process. That can be mitigated to keep the old code path and only use multiprocessing when n_processes >= 2. Overall I think this is pretty positive for my use case, the total time of running the validation steps went down from 7.5 minutes to 5.5 minutes or so. The other parts of the validation can also be split out over different processes, but that's for another time. [1]: http://www.stoq.com.br/ -- Johan Dahlin

2 2

Fwd: Spurious useless-else-on-loop warning
by Skip Montanaro 18 Sep '13

18 Sep '13

Ned, I still think you're missing the point of my original post. A return is as good as a break when considering early exit from a loop. The pylint code only checks for the presence of a break statement, but the test cases clearly show a case with a return statement. The test functions even include docstrings which state that. That's all I'm trying to point out. That I discovered the problem in a piece of code which could be rewritten to avoid the warning is quite beside the point. Skip

3 2

Fwd: Spurious useless-else-on-loop warning
by Skip Montanaro 18 Sep '13

18 Sep '13

> But this "else" is useless, isn't it? Just put the "return dt" statement after the for statement, no else needed. In this particular case, yes, but that's not the point of the post. Pylint doesn't check to see what's in the else clause, only whether there is an early exit from the body of the loop. Skip

1 0

Re: [code-quality] Spurious useless-else-on-loop warning
by Ned Batchelder 18 Sep '13

18 Sep '13

On 9/17/13 7:56 PM, Skip Montanaro wrote: > > > > But this "else" is useless, isn't it? Just put the "return dt" > statement after the for statement, no else needed. > > In this particular case, yes, but that's not the point of the post. > Pylint doesn't check to see what's in the else clause, only whether > there is an early exit from the body of the loop. > > Skip > But is there a sensible use-case for a for loop with an else but no break? I can't think of one. There's no point changing pylint unless you can find a useful piece of code that it complains about. --Ned.

1 0

Spurious useless-else-on-loop warning
by Skip Montanaro 17 Sep '13

17 Sep '13

I have a function which uses for ... else, with an early return. Here's a cut down version of it: """...""" def func(c, dt, ct): for cl, op in zip(c.events[0::2], c.events[1::2]): delta = cl.event_datetime - dt if delta > ct: return dt + ct else: dt = op.event_datetime ct -= delta else: return dt Pylint (1.0.0) complains: early_exit.py:11: [W0120(useless-else-on-loop), func] Else clause on loop without a break statement I thought to myself, "self, a return should be treated like a break," and set out to fix this problem. I found the suspect code in checkers/base.py: def _loop_exits_early(loop): """Returns true if a loop has a break statement in its body.""" loop_nodes = (astroid.For, astroid.While) # Loop over body explicitly to avoid matching break statements # in orelse. for child in loop.body: if isinstance(child, loop_nodes): continue for _ in child.nodes_of_class(astroid.Break, skip_klass=loop_nodes): return True return False Looking to see where a test case would go, I found test/input/func_useless_else_on_loop.py, with this test case: def test_return_for(): """else + return is accetable.""" for i in range(10): if i % 2: return i else: print 'math is broken' Given that _loop_exits_early doesn't check for early returns, how does test_return_for (and test_return_while, not shown) pass when the test suite is run? I think these two lines belong in _loop_exits_early: for _ in child.nodes_of_class(astroid.Return, skip_klass=loop_nodes): return True assuming there is no way to spell that with just a single call to child.nodes_of_class(). (I tried updating my repo, but can't remember my bitbucket password, otherwise I would have checked to see if this problem has already been solved. My apologies if it has.) Skip

2 1

Something about Pylint
by 侯焯明 03 Sep '13

03 Sep '13

Hello, I am a beginner to Pylint, My name is Hou Zhuoming. When I install the pylint 1.0.0 in python 3.3(OS:Windows7), there is a error happened. And then I read the source code of pylint 1.0.0. I found something is wrong in setup.py model. Line 138: call(['2to3','-wn',dest]): I think the problem is the system cannot find 2to3. But I can find a script named '2to3.py' in my python 3.3. So I change Line 138 to: call(['C:\\Python33\\Tools\\Scripts\\2to3.py', '-wn', dest], shell=sys.platform=='win32'). It works! I can install pylint 1.0.0 successfully after the change. My setup.py in pylint 1.0.0: I found the same problem in astroid-1.0.0 and logilab-common-0.60.0 too. I also can install these two packages with the same change. I am not sure it is a bug. Maybe, something is wrong when I am installing the pylint 1.0.0. (No problem in Linux) Another problem: when I successfully installed pylint 1.0.0, I want to run the pylint-gui.exe. I can see the GUI successfully. But when I want to test some python scripts, it works wrong.(The pylint.exe works normally) All these problems happen in python 3.3(Operating system is Windows 7). I need your help. Thank you. ------------------ School of Software,Sun Yat-sen University Zhuoming Hou(侯焯明) Email:745189913@qq.com

3 6