[Piglit] [PATCH 18/26] parse_glspec.py: PEP8 compliance

[Dylan Baker]

I'll fix that dict and look through the others. the PEP8 tool does complain
about it.

So, so, so. Fedora ships an old version and this is a known bug in the
version fedora ships. I just manually upraded via pip and now this does not
show up as an error. doh!

So this is completely my bad. I'll take a look for changes like this and
revert them.


On Tue, Jul 23, 2013 at 11:38 AM, Chad Versace <chad.versace at linux.intel.com
> wrote:

> On 07/23/2013 07:46 AM, Dylan Baker wrote:
>
>> I'll look through for the parens, It might be in a seperate patch, but I
>> think it deserves to be here, since PEP8 is all about readability.
>>
>
> If you want to defer the parens to a separate patch, that's ok. But, in
> this
> patch we shouldn't commit this atrocious dict. It's really unreadable.
>
>
> >>> +        return 'GLES{0}.{1}'.format(ones, tenths), {'kind': 'GLES',
> >>> +                                                    'gl_10x_version':
> >>> +                                                    10 * ones +
> tenths}
>
>
>
>> The final one it complained about, but after reading PEP8 again I
>> understand why, and can fix it. In this case it wants something like this:
>> variable = {
>>       key: value,
>>       key: value
>> }
>>
>> With the important note that the closing brace (or parentheses or bracket)
>> should either be on the last line of text, or should be on the next line
>> even with the indent of the first line.
>>
>
> Strange. PEP8 says
>
> --- -8<- ---
> The closing brace/bracket/parenthesis on multi-line constructs may either
> line up under the first non-whitespace character of the last line of list,
> as in:
>
> my_list = [
>     1, 2, 3,
>     4, 5, 6,
>     ]
> result = some_function_that_takes_**arguments(
>     'a', 'b', 'c',
>     'd', 'e', 'f',
>     )
> or it may be lined up under the first character of the line that starts
> the multi-line construct, as in:
>
> my_list = [
>     1, 2, 3,
>     4, 5, 6,
> ]
> result = some_function_that_takes_**arguments(
>     'a', 'b', 'c',
>     'd', 'e', 'f',
> )
> --- >8- ---
>
> So, the tool should not have complained about
>
> my_dict = {
>     k1: v1,
>     k2: v2,
>     }
>
> Anyways, if the pep8 tool does incorrectly complain about that dict,
> then go ahead and fix it. I'm just concerned that, even if I take
> pains to code according to PEP8, then the tool will still loudly
> complain.
>
>
>  On Mon, Jul 22, 2013 at 2:26 PM, Chad Versace
>> <chad.versace at linux.intel.com>**wrote:
>>
>>  On 07/10/2013 03:19 PM, Dylan Baker wrote:
>>>
>>>  There are a 3 lines that are just over the recommended 79 character
>>>> limit, however, breaking them made them harder to read so they were
>>>> left.
>>>> ---
>>>>    glapi/parse_glspec.py | 76 ++++++++++++++++++++++++------****
>>>>
>>>> ---------------------
>>>>    1 file changed, 36 insertions(+), 40 deletions(-)
>>>>
>>>>
>>>
>>>
>>>
>>>         m = GLES_VERSION_REGEXP.match(****category_name)
>>>>
>>>>        if m:
>>>>            ones = int(m.group(1))
>>>>            tenths = int(m.group(2))
>>>> -        return 'GLES{0}.{1}'.format(ones, tenths), {
>>>> -            'kind': 'GLES',
>>>> -            'gl_10x_version': 10 * ones + tenths
>>>> -            }
>>>> +        return 'GLES{0}.{1}'.format(ones, tenths), {'kind': 'GLES',
>>>> +                                                    'gl_10x_version':
>>>> +                                                    10 * ones + tenths}
>>>>
>>>>
>>>>
>>> The original was significantly more readable. Maybe you could achieve
>>> PEP-8
>>> correctness while maintaining readability by using explicit parens around
>>> the
>>> tuple. Maybe like this:
>>>
>>>      return ('GLES{0}.{1}'.format(ones, tenths),
>>>              {'kind': 'GLES',
>>>               'gl_10x_version': 10 * ones + tenths})
>>>
>>> or this
>>>
>>>      return (
>>>
>>>          'GLES{0}.{1}'.format(ones, tenths),
>>>          {'kind': 'GLES',
>>>           'gl_10x_version': 10 * ones + tenths}
>>>      )
>>>
>>>  From my interpretation of the PEP8 doc, both are PEP8 correct.
>>>
>>>
>>>
>>>
>>>         extension_name = 'GL_' + category_name
>>>
>>>> -    return extension_name, {
>>>> -        'kind': 'extension',
>>>> -        'extension_name': extension_name
>>>> -        }
>>>> +    return extension_name, {'kind': 'extension',
>>>> +                            'extension_name': extension_name}
>>>>
>>>>
>>>
>>> While your changing the formatting of tuples, could you add explicit
>>> parens to aid readability? There are several other locations in this
>>> patch where parens could be added. Or do you think that belongs to
>>> a separate patch?
>>>
>>>
>>>
>>>
>>>             if name not in self.functions:
>>>
>>>> -            self.functions[name] = {
>>>> -                'return_type': return_type,
>>>> -                'param_names': param_names,
>>>> -                'param_types': param_types,
>>>> -                'categories': [category],
>>>> -                }
>>>> +            self.functions[name] = {'return_type': return_type,
>>>> +                                    'param_names': param_names,
>>>> +                                    'param_types': param_types,
>>>> +                                    'categories': [category]}
>>>>
>>>>
>>> Did the PEP8 tool complain about this one?
>>>
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20130723/55cf56eb/attachment.html>