<div dir="ltr"><div>I'll fix that dict and look through the others. the PEP8 tool does complain about it. </div>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. </div><div class="gmail_extra"> <div class="gmail_quote">On Tue, Jul 23, 2013 at 11:38 AM, Chad Versace <<a href="mailto:chad.versace@linux.intel.com" target="_blank">chad.versace@linux.intel.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">On 07/23/2013 07:46 AM, Dylan Baker wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> 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. </blockquote> </div> 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.<div class="im"> >>> + return 'GLES{0}.{1}'.format(ones, tenths), {'kind': 'GLES', >>> + 'gl_10x_version': >>> + 10 * ones + tenths} </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div class="im"> 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. </div></blockquote> 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. <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im"> On Mon, Jul 22, 2013 at 2:26 PM, Chad Versace <<a href="mailto:chad.versace@linux.intel.com" target="_blank">chad.versace@linux.intel.com</a>>wrote: </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im"> On 07/10/2013 03:19 PM, Dylan Baker wrote: </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im"> 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. --- </div> glapi/parse_glspec.py | 76 ++++++++++++++++++++++++------**<div class="im"> --------------------- 1 file changed, 36 insertions(+), 40 deletions(-) </div></blockquote> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> m = GLES_VERSION_REGEXP.match(**category_name)<div><div class="h5"> 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} </div></div></blockquote><div><div class="h5"> 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 <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> - return extension_name, { - 'kind': 'extension', - 'extension_name': extension_name - } + return extension_name, {'kind': 'extension', + 'extension_name': extension_name} </blockquote> 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: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> - 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]} </blockquote> Did the PEP8 tool complain about this one? </div></div></blockquote> </blockquote> </blockquote></div> </div>