[Piglit] [PATCH 4/9] framework/exectest.py: remove deprecated builtin use

Dylan Baker baker.dylan.c at gmail.com
Wed Apr 9 19:47:01 PDT 2014


On Wednesday, April 09, 2014 09:44:28 PM Ilia Mirkin wrote:
> On Wed, Apr 9, 2014 at 9:27 PM, Dylan Baker <baker.dylan.c at gmail.com> wrote:
> > Upstream python considers map() and filter() deprecated in favor of
> > using comprehensions. These comprehensions are easier to read, and can
> > act as both a map and a filter at the same time. In this case we are
> > using generator comprehensions, which have an additional advantage of
> > lower memory consumption.
> > ---
> > 
> >  framework/exectest.py | 30 ++++++++++++++----------------
> >  1 file changed, 14 insertions(+), 16 deletions(-)
> > 
> > diff --git a/framework/exectest.py b/framework/exectest.py
> > index a670ddd..ae397f0 100644
> > --- a/framework/exectest.py
> > +++ b/framework/exectest.py
> > 
> > @@ -295,23 +295,21 @@ class PiglitTest(Test):
> >      def interpret_result(self, out, returncode, results):
> >          outlines = out.split('\n')
> > 
> > -        outpiglit = map(lambda s: s[7:],
> > -                        filter(lambda s: s.startswith('PIGLIT:'),
> > outlines)) +        outpiglit = (s[7:] for s in outlines if
> > s.startswith('PIGLIT:'))
> > 
> > -        if len(outpiglit) > 0:
> > -            try:
> > -                for piglit in outpiglit:
> > -                    if piglit.startswith('subtest'):
> > -                        if not 'subtest' in results:
> > -                            results['subtest'] = {}
> > -                        results['subtest'].update(eval(piglit[7:]))
> > -                    else:
> > -                        results.update(eval(piglit))
> > -                out = '\n'.join(filter(lambda s: not
> > s.startswith('PIGLIT:'), -                                      
> > outlines))
> > -            except:
> > -                results['result'] = 'fail'
> > -                results['note'] = 'Failed to parse result string'
> > +        try:
> > +            for piglit in outpiglit:
> > +                if piglit.startswith('subtest'):
> > +                    if not 'subtest' in results:
> > +                        results['subtest'] = {}
> > +                    results['subtest'].update(eval(piglit[7:]))
> > +                else:
> > +                    results.update(eval(piglit))
> > +            out = '\n'.join(s for s in outlines if not
> > +                            s.startswith('PIGLIT:'))
> 
> IMO it's more readable to break the line like
> 
> +            out = '\n'.join(s for s in outlines
> +                            if not s.startswith('PIGLIT:'))
> 
> That way the condition is next to the 'if' (and the not). (I presume
> it doesn't fit in 80 chars in the first place? Seems like it ought
> to...)

It would if it wasn't inside a class, inside a method, inside an if block, 
inside of a try/except block.

I go back and forth on which one's better. I'll restructure it.

> > +        except:
> > +            results['result'] = 'fail'
> > +            results['note'] = 'Failed to parse result string'
> > 
> >          if 'result' not in results:
> >              results['result'] = 'fail'
> > 
> > --
> > 1.9.1
> > 
> > _______________________________________________
> > Piglit mailing list
> > Piglit at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/piglit
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20140409/edb116f5/attachment.sig>


More information about the Piglit mailing list