[Piglit] [PATCH 20/26] gen_builtin_packing_tests.py PEP8 compliance

[Chad Versace]

On 07/10/2013 03:19 PM, Dylan Baker wrote:
> 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(-)



>   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


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.


> @@ -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))


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),
                                        valid_outputs=valid_outputs_1))




>       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"}


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.


>
>   # 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"])}
> +



> -        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))

Again, did the PEP8 tool complain about this? If so, I'm very surprised.