[pacman-dev] [PATCH 4/8] Add shims to return views on Python dicts.

[Martin Panter]

On 10 October 2013 00:35, Jeremy Heiner <scalaprotractor at gmail.com> wrote:
> Reported by 2to3. The .items, .keys, and .values methods in Python 2
> make expensive copies, so the test framework uses the .iter*
> flavors. But in Python 3 the .iter* (and even the 2.7 .view*)
> flavors are removed and the original methods return fast views.
> The shims examine the runtime's version and calls the right method to
> avoid making copies.

Perhaps it might be simpler to just use Python 3’s items() etc methods
directly. That tends to also work with Python < 3 in most cases. I
admit I’ve never used Pacman’s test suite so perhaps the extra expense
would be significant under Python 2?

Anyway unless there is a good reason to still prefer Python 2.7, I’d
say keep it simple and use plain Python 3 code.

> Signed-off-by: Jeremy Heiner <ScalaProtractor at gmail.com>
> ---
>  test/pacman/pmdb.py   |  4 ++--
>  test/pacman/pmpkg.py  |  2 +-
>  test/pacman/pmtest.py |  6 +++---
>  test/pacman/util.py   | 15 ++++++++++++---
>  4 files changed, 18 insertions(+), 9 deletions(-)
>
> diff --git a/test/pacman/pmdb.py b/test/pacman/pmdb.py
> index b7b3522..f4bc71d 100644
> --- a/test/pacman/pmdb.py
> +++ b/test/pacman/pmdb.py
> @@ -236,7 +236,7 @@ def generate(self):
>              for pkg, entry in pkg_entries:
>                  path = os.path.join(self.dbdir, pkg.fullname())
>                  util.mkdir(path)
> -                for name, data in entry.iteritems():
> +                for name, data in util.itemsview(entry):
>                      util.mkfile(path, name, data)
>
>          if self.dbfile:
> @@ -247,7 +247,7 @@ def generate(self):
>                  info = tarfile.TarInfo(pkg.fullname())
>                  info.type = tarfile.DIRTYPE
>                  tar.addfile(info)
> -                for name, data in entry.iteritems():
> +                for name, data in util.itemsview(entry):
>                      filename = os.path.join(pkg.fullname(), name)
>                      info = tarfile.TarInfo(filename)
>                      info.size = len(data)
> diff --git a/test/pacman/pmpkg.py b/test/pacman/pmpkg.py
> index 0c1ea68..69734b0 100644
> --- a/test/pacman/pmpkg.py
> +++ b/test/pacman/pmpkg.py
> @@ -225,7 +225,7 @@ def local_backup_entries(self):
>
>      def installfile(self):
>          data = []
> -        for key, value in self.install.iteritems():
> +        for key, value in util.itemsview(self.install):
>              if value:
>                  data.append("%s() {\n%s\n}\n" % (key, value))
>
> diff --git a/test/pacman/pmtest.py b/test/pacman/pmtest.py
> index 57e6538..a3e4a47 100644
> --- a/test/pacman/pmtest.py
> +++ b/test/pacman/pmtest.py
> @@ -58,7 +58,7 @@ def findpkg(self, name, version, allow_local=False):
>          """Find a package object matching the name and version specified in
>          either sync databases or the local package collection. The local database
>          is allowed to match if allow_local is True."""
> -        for db in self.db.itervalues():
> +        for db in util.valuesview(self.db):
>              if db.is_local and not allow_local:
>                  continue
>              pkg = db.getpkg(name)
> @@ -154,7 +154,7 @@ def generate(self, pacman):
>              vprint("\t%s" % os.path.join(util.TMPDIR, pkg.filename()))
>              pkg.finalize()
>              pkg.makepkg(tmpdir)
> -        for key, value in self.db.iteritems():
> +        for key, value in util.itemsview(self.db):
>              for pkg in value.pkgs:
>                  pkg.finalize()
>              if key == "local" and not self.createlocalpkgs:
> @@ -170,7 +170,7 @@ def generate(self, pacman):
>
>          # Creating sync database archives
>          vprint("    Creating databases")
> -        for key, value in self.db.iteritems():
> +        for key, value in util.itemsview(self.db):
>              vprint("\t" + value.treename)
>              value.generate()
>
> diff --git a/test/pacman/util.py b/test/pacman/util.py
> index 77dd804..107a224 100644
> --- a/test/pacman/util.py
> +++ b/test/pacman/util.py
> @@ -59,6 +59,15 @@ def vprint(msg):
>  PERM_DIR  = int("755",8) # 0o755
>  PERM_FILE = int("644",8) # 0o644
>
> +def keysview(x):
> +    return x.keys() if sys.hexversion >= 0x03000000 else x.iterkeys()
> +
> +def valuesview(x):
> +    return x.values() if sys.hexversion >= 0x03000000 else x.itervalues()
> +
> +def itemsview(x):
> +    return x.items() if sys.hexversion >= 0x03000000 else x.iteritems()
> +
>
>  #
>  # Methods to generate files
> @@ -125,13 +134,13 @@ def writedata(filename, data):
>  def mkcfgfile(filename, root, option, db):
>      # Options
>      data = ["[options]"]
> -    for key, value in option.iteritems():
> +    for key, value in itemsview(option):
>          data.extend(["%s = %s" % (key, j) for j in value])
>
>      # Repositories
>      # sort by repo name so tests can predict repo order, rather than be
>      # subjects to the whims of python dict() ordering
> -    for key in sorted(db.iterkeys()):
> +    for key in sorted(keysview(db)):
>          if key != "local":
>              value = db[key]
>              data.append("[%s]\n" \
> @@ -139,7 +148,7 @@ def mkcfgfile(filename, root, option, db):
>                      "Server = file://%s" \
>                       % (value.treename, value.getverify(), \
>                          os.path.join(root, SYNCREPO, value.treename)))
> -            for optkey, optval in value.option.iteritems():
> +            for optkey, optval in itemsview(value.option):
>                  data.extend(["%s = %s" % (optkey, j) for j in optval])
>
>      mkfile(root, filename, "\n".join(data))
> --
> 1.8.4