[Mesa-dev] [PATCH 16/26] python: Explicitly use lists

[Dylan Baker]

Quoting Mathieu Bridon (2018-07-06 02:43:46)
> On Thu, 2018-07-05 at 09:31 -0700, Dylan Baker wrote:
> > Quoting Mathieu Bridon (2018-07-05 06:17:47)
> > > On Python 2, the builtin functions filter() and zip() would return
> > > lists.
> > > 
> > > On Python 3, they return iterators.
> > > 
> > > Since we want to use those objects in contexts where we need lists,
> > > we
> > > need to explicitly turn them into lists.
> > > 
> > > This makes the code compatible with both Python 2 and Python 3.
> > > 
> > > Signed-off-by: Mathieu Bridon <bochecha at daitauha.fr>
> > > ---
> > >  src/compiler/nir/nir_opt_algebraic.py | 2 +-
> > >  src/mesa/main/get_hash_generator.py   | 4 ++--
> > >  2 files changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/src/compiler/nir/nir_opt_algebraic.py
> > > b/src/compiler/nir/nir_opt_algebraic.py
> > > index 5e07d662b0..7b2ba56990 100644
> > > --- a/src/compiler/nir/nir_opt_algebraic.py
> > > +++ b/src/compiler/nir/nir_opt_algebraic.py
> > > @@ -633,7 +633,7 @@ optimizations = [
> > >  
> > >  invert = OrderedDict([('feq', 'fne'), ('fne', 'feq'), ('fge',
> > > 'flt'), ('flt', 'fge')])
> > >  
> > > -for left, right in list(itertools.combinations(invert.keys(), 2))
> > > + zip(invert.keys(), invert.keys()):
> > > +for left, right in list(itertools.combinations(invert.keys(), 2))
> > > + list(zip(invert.keys(), invert.keys())):
> > 
> > Isn't this just a really expenisve re-implementation of:
> > itertools.combinations_with_replacement(invert.keys(), 2)
> 
> It seems to be, yes. (I was so focused on fixing the "can't concatenate
> list with zip" error that I completely missed it)
> 
> However, since dict.keys() isn't guaranteed to always be in the same
> order, it is theoretically possible that:

ah, but invert is an OrderedDict, so the ordering is guaranteed :)

> 
> >>> zip(invert.keys(), invert.keys())
> 
> doesn't always return the same thing.
> 
> That is, the following could happen:
> 
> >>> d = {'A': \u2026, 'B': \u2026}
> >>> zip(d.keys(), d.keys())
> ['AB', 'BA']
> 
> Which would make the whole line not equivalent to
> `itertools.combinations_with_replacement()` any more.
> 
> In practice it probably doesn't happen though, but that means if the
> intention behind the code was what you suspect, then by using
> `itertools.combinations_with_replacement()` we'd be fixing an actual
> bug, not just making the script compatible with Python 3. :)
> 
> 
> -- 
> Mathieu
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: signature
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180807/7b04d338/attachment.sig>