[Python-Dev] [Python-checkins] cpython: Modernize modulefinder module and tests a bit.

[Ezio Melotti]

Hi,

On 29/07/2011 15.35, eric.araujo wrote:
> http://hg.python.org/cpython/rev/1521d9837d16
> changeset:   71569:1521d9837d16
> user:        Éric Araujo<merwok at netwok.org>
> date:        Thu Jul 28 23:35:29 2011 +0200
> summary:
>    Modernize modulefinder module and tests a bit.
>
> The tests don’t use an internal distutils function anymore, and use
> regular assertEqual with sorted lists instead of a convoluted manual
> diff.
>
> files:
>    Lib/modulefinder.py           |  15 ++----
>    Lib/test/test_modulefinder.py |  48 +++++++++++-----------
>    2 files changed, 31 insertions(+), 32 deletions(-)
>
>
> diff --git a/Lib/modulefinder.py b/Lib/modulefinder.py
> --- a/Lib/modulefinder.py
> +++ b/Lib/modulefinder.py
> @@ -1,6 +1,5 @@
>   """Find modules used by a script, using introspection."""
>
> -from __future__ import generators
>   import dis
>   import imp
>   import marshal
> @@ -9,8 +8,6 @@
>   import types
>   import struct
>
> -READ_MODE = "rU"
> -
>   # XXX Clean up once str8's cstor matches bytes.
>   LOAD_CONST = bytes([dis.opname.index('LOAD_CONST')])
>   IMPORT_NAME = bytes([dis.opname.index('IMPORT_NAME')])
> @@ -29,8 +26,7 @@
>
>   # A Public interface
>   def AddPackagePath(packagename, path):
> -    paths = packagePathMap.get(packagename, [])
> -    paths.append(path)
> +    paths = packagePathMap.setdefault(packagename, []).append(path)

I'm assuming that packagePathMap is a dict that might contain or not a 
*packagename* key that maps to a list of paths.
Now, unless I'm missing something, the old code assigned to *paths* the 
list of paths or [] if it wasn't there, and then appended *path* to it.
AFAICS, the new code introduced two changes:
   1) the packagename key is added to the dict if it was missing -- and 
this seems reasonable;
   2) append is now on the same line, it returns None, and None is 
assigned to *paths* -- and this seems wrong;

>       packagePathMap[packagename] = paths

Also this is not necessary anymore if you use setdefault.

>   replacePackageMap = {}
> @@ -106,14 +102,14 @@
>
>   [...]

Best Regards,
Ezio Melotti