<div dir="ltr"><div><div>1) I'll change that </div>2) I'll fix that too </div>3) I commented on what's wrong with that in a previous patch, I'll change it to be more like the original </div><div class="gmail_extra"> <div class="gmail_quote">On Mon, Jul 22, 2013 at 2:41 PM, 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/10/2013 03:19 PM, Dylan Baker wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> A few errors according to the pep8 tool were left behind. 1255-1257: I believe these to be false positives, my reading of PEP 8 makes them seem correct, and the tool does not complain about other similar lines 247, 392: These are part of a mako template that is embedded in the python file. A better solution than wrapping them would be to split them out of the python and put them in their own mako files. --- generated_tests/gen_builtin_packing_tests.py | 311 ++++++++++++++------------- 1 file changed, 161 insertions(+), 150 deletions(-) </blockquote> </div><div><div class="h5"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> def make_inputs_for_unpack_half_2x16(): # For each of the two classes of float16 values, subnormal and normalized, # below are listed the exponent and mantissa of the class's boundary # values and some values slightly inside the bounds. - bounds = ( - ( 0, 0), # zero - - ( 0, 1), # subnormal_min - ( 0, 2), # subnormal_min + min_step - - ( 0, 1022), # subnormal_max - min_step - ( 0, 1023), # subnormal_max - - ( 1, 0), # normal_min - ( 1, 1), # normal_min + min_step - - (30, 1022), # normal_max - max_step - (30, 1023), # normal_max - - (31, 0), # inf - ) + bounds = ((0, 0), # zero + (0, 1), # subnormal_min + (0, 2), # subnormal_min + min_step + (0, 1022), # subnormal_max - min_step + (0, 1023), # subnormal_max + (1, 0), # normal_min + (1, 1), # normal_min + min_step + (30, 1022), # normal_max - max_step + (30, 1023), # normal_max + (31, 0)) # inf </blockquote> </div></div> The original was a table with each cell right-justified. It's ok if you want to remove the spurious empty lines, but please preserve the tabular formatting.<div class="im"> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> @@ -1218,51 +1230,58 @@ def make_inouts_for_unpack_4x8(unpack_1x8_func, uint8_inputs): glsl_literal(vec4_1[2]), glsl_literal(vec4_1[3]))) - inout_seq.append( - InOutTuple(input=glsl_literal(u32_0), - valid_outputs=valid_outputs_0)) - inout_seq.append( - InOutTuple(input=glsl_literal(u32_1), - valid_outputs=valid_outputs_1)) + inout_seq.append(InOutTuple(input=glsl_literal(u32_0), + valid_outputs=valid_outputs_0)) + inout_seq.append(InOutTuple(input=glsl_literal(u32_1), + valid_outputs=valid_outputs_1)) </blockquote> </div> This looks completely wrong. However, this looks right: inout_seq.append(InOutTuple(input=glsl_literal(u32_0), valid_outputs=valid_outputs_0)) inout_seq.append(InOutTuple(input=glsl_literal(u32_1),<div><div class="h5"> valid_outputs=valid_outputs_1)) <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> return inout_seq # This table maps GLSL pack/unpack function names to the precision of their # return type. -result_precision_table = { - "packSnorm2x16": "highp", - "packSnorm4x8": "highp", - "packUnorm2x16": "highp", - "packUnorm4x8": "highp", - "packHalf2x16": "highp", - - "unpackSnorm2x16": "highp", - "unpackSnorm4x8": "highp", - "unpackUnorm2x16": "highp", - "unpackUnorm4x8": "highp", - "unpackHalf2x16": "mediump", - } +result_precision_table = {"packSnorm2x16": "highp", + "packSnorm4x8": "highp", + "packUnorm2x16": "highp", + "packUnorm4x8": "highp", + "packHalf2x16": "highp", + "unpackSnorm2x16": "highp", + "unpackSnorm4x8": "highp", + "unpackUnorm2x16": "highp", + "unpackUnorm4x8": "highp", + "unpackHalf2x16": "mediump"} </blockquote> </div></div> Did the PEP8 tool complain about this? The original dict was PEP8-compliant, except maybe for the spurious newline and accidental double-space after *packHalf2x16.<div><div class="h5"> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> # This table maps GLSL pack/unpack function names to a sequence of InOutTuple. -inout_table = { - "packSnorm2x16": make_inouts_for_pack_2x16(pack_snorm_1x16, full_input_table["packSnorm2x16"], reduced_input_table["packSnorm2x16"]), - "packSnorm4x8": make_inouts_for_pack_4x8(pack_snorm_1x8, full_input_table["packSnorm4x8"]), - "packUnorm2x16": make_inouts_for_pack_2x16(pack_unorm_1x16, full_input_table["packUnorm2x16"], reduced_input_table["packUnorm2x16"]), - "packUnorm4x8": make_inouts_for_pack_4x8(pack_unorm_1x8, full_input_table["packUnorm4x8"]), - "packHalf2x16": make_inouts_for_pack_2x16(pack_half_1x16, full_input_table["packHalf2x16"], reduced_input_table["packHalf2x16"]), - - - "unpackSnorm2x16": make_inouts_for_unpack_2x16(unpack_snorm_1x16, full_input_table["unpackSnorm2x16"]), - "unpackSnorm4x8": make_inouts_for_unpack_4x8(unpack_snorm_1x8, full_input_table["unpackSnorm4x8"]), - "unpackUnorm2x16": make_inouts_for_unpack_2x16(unpack_unorm_1x16, full_input_table["unpackUnorm2x16"]), - "unpackUnorm4x8": make_inouts_for_unpack_4x8(unpack_unorm_1x8, full_input_table["unpackUnorm4x8"]), - "unpackHalf2x16": make_inouts_for_unpack_2x16(unpack_half_1x16, full_input_table["unpackHalf2x16"]), - } +inout_table = {"packSnorm2x16": make_inouts_for_pack_2x16( + pack_snorm_1x16, + full_input_table["packSnorm2x16"], + reduced_input_table["packSnorm2x16"]), + "packSnorm4x8": make_inouts_for_pack_4x8( + pack_snorm_1x8, full_input_table["packSnorm4x8"]), + "packUnorm2x16": make_inouts_for_pack_2x16( + pack_unorm_1x16, full_input_table["packUnorm2x16"], + reduced_input_table["packUnorm2x16"]), + "packUnorm4x8": make_inouts_for_pack_4x8( + pack_unorm_1x8, full_input_table["packUnorm4x8"]), + "packHalf2x16": make_inouts_for_pack_2x16( + pack_half_1x16, full_input_table["packHalf2x16"], + reduced_input_table["packHalf2x16"]), + "unpackSnorm2x16": make_inouts_for_unpack_2x16( + unpack_snorm_1x16, full_input_table["unpackSnorm2x16"]), + "unpackSnorm4x8": make_inouts_for_unpack_4x8( + unpack_snorm_1x8, full_input_table["unpackSnorm4x8"]), + "unpackUnorm2x16": make_inouts_for_unpack_2x16( + unpack_unorm_1x16, full_input_table["unpackUnorm2x16"]), + "unpackUnorm4x8": make_inouts_for_unpack_4x8( + unpack_unorm_1x8, full_input_table["unpackUnorm4x8"]), + "unpackHalf2x16": make_inouts_for_unpack_2x16( + unpack_half_1x16, full_input_table["unpackHalf2x16"])} + </blockquote> </div></div><div class="im"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> - glsl_es_300_funcs = ( - FuncInfo("packSnorm2x16", requirements), - FuncInfo("packUnorm2x16", requirements), - FuncInfo("packHalf2x16", requirements), - - FuncInfo("unpackSnorm2x16", requirements), - FuncInfo("unpackUnorm2x16", requirements), - FuncInfo("unpackHalf2x16", requirements), - ) + glsl_es_300_funcs = (FuncInfo("packSnorm2x16", requirements), + FuncInfo("packUnorm2x16", requirements), + FuncInfo("packHalf2x16", requirements), + FuncInfo("unpackSnorm2x16", requirements), + FuncInfo("unpackUnorm2x16", requirements), + FuncInfo("unpackHalf2x16", requirements)) </blockquote> </div> Again, did the PEP8 tool complain about this? If so, I'm very surprised. </blockquote></div> </div>