[Piglit] [PATCH 16/45] gen_shader_bit_encoding: cleanup some style issues

[Ilia Mirkin]

On Fri, Nov 14, 2014 at 6:37 PM, Dylan Baker <baker.dylan.c at gmail.com> wrote:
> On Friday, November 14, 2014 02:04:23 PM Matt Turner wrote:
>> On Wed, Nov 12, 2014 at 3:45 PM, Dylan Baker <baker.dylan.c at gmail.com> wrote:
>> > Signed-off-by: Dylan Baker <dylanx.c.baker at intel.com>
>> > ---
>> >  generated_tests/gen_shader_bit_encoding_tests.py | 142 ++++++++++++-----------
>> >  1 file changed, 76 insertions(+), 66 deletions(-)
>> >
>> > diff --git a/generated_tests/gen_shader_bit_encoding_tests.py b/generated_tests/gen_shader_bit_encoding_tests.py
>> > index 84b9390..d409db0 100644
>> > --- a/generated_tests/gen_shader_bit_encoding_tests.py
>> > +++ b/generated_tests/gen_shader_bit_encoding_tests.py
>> > @@ -29,31 +29,42 @@ from templates import template_file
>> >  TEMPLATE = template_file(os.path.basename(os.path.splitext(__file__)[0]),
>> >                           'template.shader_test.mako')
>> >
>> > -
>> > -def floatBitsToInt(f):
>> > +def floatbitstoint(f):
>>
>> These were named with capital letters to match the GLSL built-in
>> function names. I don't see any reason to change them.
>>
>
> Because in python functions don't have capital letters in them? I
> understand that these function emulate the behavior of GLSL functions,
> but this is python.

I think the #1 directive should be not to comply with some document
that has the "correct" way of writing python, but instead to make
things readable. And when in doubt, follow what the document says. In
this case, floatBitsToInt is clearly superior to floatbitstoint. You
could change it to float_bits_to_int or something, at which point it's
debatable.

>
>> >      return struct.unpack('i', struct.pack('f', f))[0]
>> >
>> > -def floatBitsToUint(f):
>> > +
>> > +def floatbitstouint(f):
>> >      return struct.unpack('I', struct.pack('f', f))[0]
>> >
>> > -def intBitsToFloat(i):
>> > +
>> > +def intbitstofloat(i):
>> >      return struct.unpack('f', struct.pack('i', i))[0]
>> >
>> > -def uintBitsToFloat(u):
>> > +
>> > +def uintbitstofloat(u):
>> >      return struct.unpack('f', struct.pack('I', u))[0]
>> >
>> > +
>> >  def passthrough(f):
>> >      return f
>> >
>> > +
>> >  def neg(num):
>> >      return -num
>> >
>> > +
>> >  def neg_abs(num):
>> >      return -abs(num)
>> >
>> > +
>> >  def vec4(f):
>> >      return [f, f, f, f]
>> >
>> > +
>> > +# Don't test +inf or -inf, since we don't have a way to pass them via
>> > +# shader_runner [test] sections. Don't test NaN, since it has many
>> > +# representations. Don't test subnormal values, since hardware might
>> > +# flush them to zero.
>> >  test_data = {
>> >      # Interesting floating-point inputs
>> >      'mixed':                        (2.0, 9.5, -4.5, -25.0),
>> > @@ -65,11 +76,6 @@ test_data = {
>> >      'normalized smallest negative': vec4(-1.1754944e-38),
>> >      'normalized largest':           vec4( 3.4028235e+38),
>> >      'normalized largest negative':  vec4(-3.4028235e+38)
>> > -
>> > -    # Don't test +inf or -inf, since we don't have a way to pass them via
>> > -    # shader_runner [test] sections. Don't test NaN, since it has many
>> > -    # representations. Don't test subnormal values, since hardware might
>> > -    # flush them to zero.
>>
>> This comment was here (i.e., in the block) because that's where you'd
>> have expected to find entries for inf and NaN.
>
> Having a block comment in a dictionary looks really strange to me, but
> okay, I'll put it back.
>
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
>