[Python-Dev] 2.2a1 patch from PyChecker

[Guido van Rossum]

> Attached is a patch to address some problems found by PyChecker.
> Mostly unused imports/locals, etc.

Thanks!

> There were a few methods that were removed because they were duplicated
> (ConfigParser.py:has_option, gzip.py:writelines, locale.py:set_locale).
> I deleted the first occurrence, but I'm not sure that was always right.

It'll take us some time to review all the patches.

> I have changed many unused locals to _ (e.g., for _ in range(3):).
> This gets PyChecker to shut up about being unused.  It also defaults
> to ignoring unused local variables if their name is 'empty' or 'unused'.

Argh!  I don't like using '_' for this, because '_' is already a
special built-in in interactive mode.  I don't even like the fact that
PyChecker warns about unused loop control variables at all -- I see
nothing wrong with code like

    for i in range(10): print "X"

and I would rather not have to write it differently specifically to
shut up PyChecker.  (In general, I find it annoying when a tool like
this requires one to change one's perfectly fine coding style because
of a blind spot in the tool. :-)

I'd be happy if there was an option "complain if a loop control
variable is not used" but it was off by default.

I

> There are many more unused warnings.  Does anybody want me to change
> the variables to empty, unused, _, or something different?  

No.

> Should the default unused names be expanded?

I tend to use "dummy" when I need a dummy variable.  But even in tuple
unpacks, I prefer to give all variables meaningful names even if I
don't use all.  To me, this:

    firstname, lastname, address, phone = get_employee()

is more readable than this:

    firstname, lastname, dummy, dummy = get_employee()

> Note:  I could not run under Python 2.2a1 because dir([]) does not
> return the methods.

But you can treat it as an instance, and get the methods by looking at
[].__class__.__dict__.  Read PEP 252 -- it explains how the
introspection interface will be rationalized.

> Here are some other warnings I didn't fix because I wasn't sure what
> should be done:
> 
> audiodev.py:214: No global (BUFFERSIZE) found
> 	(this is old, should BUFFERSIZE be defined, a better exception
> 	 raised, and/or deprecation warning?)

I still haven't figured that one out myself. :-(

> cgi.py:820: Overriden method (values) doesn't match signature in
> class (cgi.FormContentDict)

I'm afraid that one will have to remain -- it exists for b/w
compatibility (with 6 year old code :-).

> ihooks.py:491: Overriden method (reload) doesn't match signature in
> class (ihooks.BasicModuleImporter)

This is intentional.

> imaplib.py:1026: No global (j) found

Huh?  There's no 'j' on that line.

> ntpath.py:411: No global (WindowsError) found
> 	(I ran on unix, not sure if this is ok on windows.)

Yes, this exception is unique to Windows.

> pdb.py:138: Overriden method (default) doesn't match signature in
> class (cmd.Cmd)

This one I don't understand.  BTW, it would be nice if you could also
mention the file and line number of the other reference.

> profile.py:179: No global (operator) found

That looks like a bug in profile.py to me: it is clearly hoping
for "import operator". :-)

But in general: thanks for PyCecker, Neil, and thanks for the patch
set!

--Guido van Rossum (home page: http://www.python.org/~guido/)