[Piglit] [PATCH] registry/gl.py: Fix PEP 8 issues.

Dylan Baker baker.dylan.c at gmail.com
Tue Mar 10 10:56:19 PDT 2015


On Mon, Mar 09, 2015 at 11:28:25PM -0700, Vinson Lee wrote:
> Signed-off-by: Vinson Lee <vlee at freedesktop.org>
> ---
>  registry/gl.py | 106 +++++++++++++++++++++++++++++++++++++++------------------
>  1 file changed, 72 insertions(+), 34 deletions(-)
> 
> diff --git a/registry/gl.py b/registry/gl.py
> index ff89189..5cc37b5 100644
> --- a/registry/gl.py
> +++ b/registry/gl.py
> @@ -74,8 +74,8 @@ def _repair_xml(xml_registry):
>          remove_queue.append((parent, child))
>  
>      for enums in xml_registry.iterfind('./enums'):
> -        if ('GL_ALL_ATTRIB_BITS' in fixes
> -            and enums.get('group') == 'AttribMask'):
> +        if ('GL_ALL_ATTRIB_BITS' in fixes and
> +                enums.get('group') == 'AttribMask'):

If you're going to fix the first issue you need to fix the second. The
problem here is that the indents are swapped, the second line of the if
needs to have one more unit of indent, the body of the statement needs
to have one less.

ex:
if (long line and
        more stuff):
    continue

>                  # The XML defines GL_ALL_ATTRIB_BITS incorrectly with all bits
>                  # set (0xFFFFFFFF). From the GL_ARB_multisample spec, v5:
>                  #
> @@ -89,8 +89,8 @@ def _repair_xml(xml_registry):
>                  fixes.remove('GL_ALL_ATTRIB_BITS')
>                  continue
>  
> -        if ('glOcclusionQueryEventMaskAMD' in fixes
> -            and enums.get('namespace') == 'OcclusionQueryEventMaskAMD'):
> +        if ('glOcclusionQueryEventMaskAMD' in fixes and
> +                enums.get('namespace') == 'OcclusionQueryEventMaskAMD'):
>                  # This tag's attributes are totally broken.
>                  enums.set('namespace', 'GL')
>                  enums.set('group', 'OcclusionQueryEventMaskAMD')
> @@ -99,10 +99,13 @@ def _repair_xml(xml_registry):
>                  fixes.remove('glOcclusionQueryEventMaskAMD')
>                  continue
>  
> -        if ('gles2_GL_ACTIVE_PROGRAM_EXT' in fixes
> -            and enums.get('vendor') is not None and enums.get('vendor') == 'ARB'
> -            and enums.get('start') is not None and enums.get('start') <= '0x8259'
> -            and enums.get('end') is not None and enums.get('end') >= '0x8259'):
> +        if ('gles2_GL_ACTIVE_PROGRAM_EXT' in fixes and
> +                enums.get('vendor') is not None and
> +                enums.get('vendor') == 'ARB' and
> +                enums.get('start') is not None and
> +                enums.get('start') <= '0x8259' and
> +                enums.get('end') is not None and
> +                enums.get('end') >= '0x8259'):
>                  # GL_ACTIVE_PROGRAM_EXT has different numerical values in GL
>                  # (0x8B8D) and in GLES (0x8259). Remove the GLES value to avoid
>                  # redefinition collisions.
> @@ -554,7 +557,8 @@ class Extension(object):
>                  return True
>              else:
>                  return False
> -        elif (other.vendor_namespace == 'EXT') != (self.vendor_namespace == 'EXT'):
> +        elif (other.vendor_namespace == 'EXT') != \
> +                (self.vendor_namespace == 'EXT'):
>              # Sort EXT before others
>              if self.vendor_namespace == 'EXT':
>                  return True
> @@ -677,9 +681,13 @@ class CommandParam(object):
>          #
>          #    <param>const <ptype>GLchar</ptype> *<name>name</name></param>
>          #    <param len="1"><ptype>GLsizei</ptype> *<name>length</name></param>
> -        #    <param len="bufSize"><ptype>GLint</ptype> *<name>values</name></param>
> +        #    <param len="bufSize">
> +        #        <ptype>GLint</ptype> *<name>values</name>
> +        #    </param>
>          #    <param><ptype>GLenum</ptype> <name>shadertype</name></param>
> -        #    <param group="sync"><ptype>GLsync</ptype> <name>sync</name></param>
> +        #    <param group="sync">
> +        #        <ptype>GLsync</ptype> <name>sync</name>
> +        #    </param>
>          #    <param><ptype>GLuint</ptype> <name>baseAndCount</name>[2]</param>
>  
>          assert xml_param.tag == 'param'
> @@ -691,11 +699,15 @@ class CommandParam(object):
>  
>          # Parse the C type.
>          c_type_text = list(xml_param.itertext())
> -        c_type_text_end = c_type_text.pop(-1)  # Could be <name> or <array_suffix>
> -        if c_type_text_end.startswith('['): # We popped off <array_suffix>
> +
> +        # Could be <name> or <array_suffix>
> +        c_type_text_end = c_type_text.pop(-1)
> +
> +        # We popped off <array_suffix>
> +        if c_type_text_end.startswith('['):
>              # This is an array variable.
>              self.array_suffix = c_type_text_end
> -            c_type_text.pop(-1) # Pop off the next one (<name>)
> +            c_type_text.pop(-1)  # Pop off the next one (<name>)
>          else:
>              self.array_suffix = ''
>          c_type_text = (t.strip() for t in c_type_text)
> @@ -736,17 +748,32 @@ class Command(object):
>          #
>          #    <command>
>          #        <proto>void <name>glTexSubImage2D</name></proto>
> -        #        <param group="TextureTarget"><ptype>GLenum</ptype> <name>target</name></param>
> -        #        <param group="CheckedInt32"><ptype>GLint</ptype> <name>level</name></param>
> -        #        <param group="CheckedInt32"><ptype>GLint</ptype> <name>xoffset</name></param>
> -        #        <param group="CheckedInt32"><ptype>GLint</ptype> <name>yoffset</name></param>
> +        #        <param group="TextureTarget">
> +        #            <ptype>GLenum</ptype> <name>target</name>
> +        #        </param>
> +        #        <param group="CheckedInt32">
> +        #            <ptype>GLint</ptype> <name>level</name>
> +        #        </param>
> +        #        <param group="CheckedInt32">
> +        #            <ptype>GLint</ptype> <name>xoffset</name>
> +        #        </param>
> +        #        <param group="CheckedInt32">
> +        #            <ptype>GLint</ptype> <name>yoffset</name>
> +        #        </param>
>          #        <param><ptype>GLsizei</ptype> <name>width</name></param>
>          #        <param><ptype>GLsizei</ptype> <name>height</name></param>
> -        #        <param group="PixelFormat"><ptype>GLenum</ptype> <name>format</name></param>
> -        #        <param group="PixelType"><ptype>GLenum</ptype> <name>type</name></param>
> -        #        <param len="COMPSIZE(format,type,width,height)">const void *<name>pixels</name></param>
> +        #        <param group="PixelFormat">
> +        #            <ptype>GLenum</ptype> <name>format</name>
> +        #        </param>
> +        #        <param group="PixelType">
> +        #            <ptype>GLenum</ptype> <name>type</name>
> +        #        </param>
> +        #        <param len="COMPSIZE(format,type,width,height)">const void *
> +        #            <name>pixels</name>
> +        #        </param>
>          #        <glx type="render" opcode="4100"/>
> -        #        <glx type="render" opcode="332" name="glTexSubImage2DPBO" comment="PBO protocol"/>
> +        #        <glx type="render" opcode="332" name="glTexSubImage2DPBO"
> +        #             comment="PBO protocol"/>
>          #    </command>
>          #
>  
> @@ -761,9 +788,12 @@ class Command(object):
>          # Parse the return type from the <proto> element.
>          #
>          # Example of a difficult <proto> element:
> -        #     <proto group="String">const <ptype>GLubyte</ptype> *<name>glGetStringi</name></proto>
> +        #     <proto group="String">const <ptype>GLubyte</ptype> *
> +        #         <name>glGetStringi</name>
> +        #     </proto>
>          c_return_type_text = list(xml_proto.itertext())
> -        c_return_type_text.pop(-1)  # Pop off the text from the <name> subelement.
> +        # Pop off the text from the <name> subelement.
> +        c_return_type_text.pop(-1)
>          c_return_type_text = (t.strip() for t in c_return_type_text)
>          self.c_return_type = ' '.join(c_return_type_text).strip()
>  
> @@ -816,7 +846,8 @@ class Command(object):
>      @property
>      def c_prototype(self):
>          """For example, "void glAccum(GLenum o, GLfloat value)"."""
> -        return '{self.c_return_type} {self.name}({self.c_named_param_list})'.format(self=self)
> +        return '{self.c_return_type} {self.name}({self.c_named_param_list})'\
> +            .format(self=self)
>  
>      @property
>      def c_funcptr_typedef(self):
> @@ -827,7 +858,8 @@ class Command(object):
>      def c_named_param_list(self):
>          """For example, "GLenum op, GLfloat value" for glAccum."""
>          return ', '.join(
> -            '{param.c_type} {param.name}{param.array_suffix}'.format(param=param)
> +            '{param.c_type} {param.name}{param.array_suffix}'
> +            .format(param=param)

I really don't like wrapper a whole method onto a new line. I know it's
valid but its weird and hard to read. A better solution might be
something like:
'{p.c_type} {p.name}{p.array_suffix}'.format(p=param)

>              for param in self.param_list
>          )
>  
> @@ -909,7 +941,8 @@ class CommandAliasMap(object):
>      def __iter__(self):
>          """A sorted iterator over the map's unique CommandAliasSet values."""
>          if self.__sorted_unique_values is None:
> -            self.__sorted_unique_values = sorted(set(six.itervalues(self.__map)))
> +            self.__sorted_unique_values = \
> +                sorted(set(six.itervalues(self.__map)))
>  
>          return iter(self.__sorted_unique_values)
>  
> @@ -918,7 +951,8 @@ class CommandAliasMap(object):
>  
>      def add(self, command):
>          assert isinstance(command, Command)
> -        _log_debug('adding command {0!r} to CommandAliasMap'.format(command.name))
> +        _log_debug('adding command {0!r} to CommandAliasMap'
> +                   .format(command.name))

Again, I think this is really hard to read, either wrap the arguments to
format, or the whole string please.

>  
>          name = command.name
>          name_set = self.get(name, None)
> @@ -1013,13 +1047,16 @@ class EnumGroup(object):
>          # Example of a bitmask group:
>          #
>          #     <enums namespace="GL" group="SyncObjectMask" type="bitmask">
> -        #         <enum value="0x00000001" name="GL_SYNC_FLUSH_COMMANDS_BIT"/>
> -        #         <enum value="0x00000001" name="GL_SYNC_FLUSH_COMMANDS_BIT_APPLE"/>
> +        #         <enum value="0x00000001"
> +        #               name="GL_SYNC_FLUSH_COMMANDS_BIT"/>
> +        #         <enum value="0x00000001"
> +        #               name="GL_SYNC_FLUSH_COMMANDS_BIT_APPLE"/>
>          #     </enums>
>          #
>          # Example of a group that resides in OpenGL's default enum namespace:
>          #
> -        #     <enums namespace="GL" start="0x0000" end="0x7FFF" vendor="ARB" comment="...">
> +        #     <enums namespace="GL" start="0x0000" end="0x7FFF" vendor="ARB"
> +        #            comment="...">
>          #         <enum value="0x0000" name="GL_POINTS"/>
>          #         <enum value="0x0001" name="GL_LINES"/>
>          #         <enum value="0x0002" name="GL_LINE_LOOP"/>
> @@ -1131,13 +1168,14 @@ class Enum(object):
>      def __eq__(self, other):
>          if self.num_value != other.num_value:
>              return False
> -        elif (self.vendor_namespace is None) != (other.vendor_namespace is None):
> +        elif (self.vendor_namespace is None) != \
> +                (other.vendor_namespace is None):
>              return False
>          elif (self.vendor_namespace in Extension.RATIFIED_NAMESPACES) != \
> -                 (other.vendor_namespace in Extension.RATIFIED_NAMESPACES):
> +                (other.vendor_namespace in Extension.RATIFIED_NAMESPACES):
>              return False
>          elif (self.vendor_namespace == 'EXT') != \
> -                 (other.vendor_namespace == 'EXT'):
> +                (other.vendor_namespace == 'EXT'):
>              return False
>          elif self.name != other.name:
>              return False
> -- 
> 2.3.2
> 
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20150310/952acaa0/attachment.sig>


More information about the Piglit mailing list