[PATCH 3/3] pwclient: basic python3 support

Finucane, Stephen stephen.finucane at intel.com
Sat Oct 17 10:16:45 AEDT 2015


> On 16 Oct 2015 16:01, Brian Norris wrote:
> > On Fri, Oct 16, 2015 at 10:49:04PM +0000, Finucane, Stephen wrote:
> > > > Looks good to me, though I'm still a little green on Python 2/3
> > > > compatibility. I've been runnning this (plus other patches) since you
> > >
> > > What are your thoughts on the use of 'six' for this stuff? Would it
> help you here?
> >
> > I'm not very familiar, but before reading this email I was trying to
> > figure out the "best" interoperable way to replace dict.iteritems(), and
> > I see we could use six.iteritems(). So my initial review says "sure"!
> 
> dict.items() should work fine in both.  the question becomes whether the
> dict
> is so large that coercing it to a list first adds significant overhead.
> i've
> found that generally it doesn't.

`list(dict.items())` to keep it compatible with Python 3, right? It's safe to say that exceptionally few developers are going to have machines with <512MB RAM today: we're good here.

> > Except then, we rely on PyPy and nonstandard libraries which doesn't
> > seem too good to me for such a small utility :(
> 
> six is a pretty common module.  i wouldn't worry about requiring it.
> -mike

Nah, I think he's right here. At least for the server it's packaged with Django but for users downloading pwclient from the server all they get is a single executable file. It's another barrier to entry if they need to install six before they can use it. I'm going to investigate (emphasis on investigate haha) the possibility of moving pwclient out of tree so it might be an option then but otherwise I guess we should stick to the hard 'if VERSION' checks, ugly though they may be.

Stephen


More information about the Patchwork mailing list