[PATCH 0/4] qapi: Bye-bye Python 2

[PATCH 0/4] qapi: Bye-bye Python 2

[Markus Armbruster]

Markus Armbruster (4):
  qapi: Inheriting from object is pointless with Python 3, drop
  qapi: Drop conditionals for Python 2
  qapi: Use super() now we have Python 3
  qapi: Brush off some (py)lint

 scripts/qapi/commands.py       |  6 +--
 scripts/qapi/common.py         |  6 +--
 scripts/qapi/error.py          |  4 +-
 scripts/qapi/events.py         |  4 +-
 scripts/qapi/expr.py           |  3 +-
 scripts/qapi/gen.py            | 27 +++++++------
 scripts/qapi/introspect.py     |  6 +--
 scripts/qapi/parser.py         | 20 ++++------
 scripts/qapi/schema.py         | 71 +++++++++++++++++-----------------
 scripts/qapi/source.py         |  4 +-
 scripts/qapi/types.py          |  4 +-
 scripts/qapi/visit.py          |  4 +-
 tests/qapi-schema/test-qapi.py |  6 +--
 13 files changed, 73 insertions(+), 92 deletions(-)

-- 
2.21.1

[PATCH 1/4] qapi: Inheriting from object is pointless with Python 3, drop

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi/gen.py    |  2 +-
 scripts/qapi/parser.py |  6 +++---
 scripts/qapi/schema.py | 12 ++++++------
 scripts/qapi/source.py |  4 ++--
 4 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
index 95afae0615..a53a705c73 100644
--- a/scripts/qapi/gen.py
+++ b/scripts/qapi/gen.py
@@ -22,7 +22,7 @@ from qapi.common import *
 from qapi.schema import QAPISchemaVisitor
 
 
-class QAPIGen(object):
+class QAPIGen:
 
     def __init__(self, fname):
         self.fname = fname
diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index 342792e410..2e3a3c5d76 100644
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -23,7 +23,7 @@ from qapi.error import QAPIParseError, QAPISemError
 from qapi.source import QAPISourceInfo
 
 
-class QAPISchemaParser(object):
+class QAPISchemaParser:
 
     def __init__(self, fname, previously_included=None, incl_info=None):
         previously_included = previously_included or set()
@@ -293,7 +293,7 @@ class QAPISchemaParser(object):
         raise QAPIParseError(self, "documentation comment must end with '##'")
 
 
-class QAPIDoc(object):
+class QAPIDoc:
     """
     A documentation comment block, either definition or free-form
 
@@ -312,7 +312,7 @@ class QAPIDoc(object):
     Free-form documentation blocks consist only of a body section.
     """
 
-    class Section(object):
+    class Section:
         def __init__(self, name=None):
             # optional section name (argument/member or section name)
             self.name = name
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 5100110fa2..c8bcfe2c49 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -24,7 +24,7 @@ from qapi.expr import check_exprs
 from qapi.parser import QAPISchemaParser
 
 
-class QAPISchemaEntity(object):
+class QAPISchemaEntity:
     meta = None
 
     def __init__(self, name, info, doc, ifcond=None, features=None):
@@ -89,7 +89,7 @@ class QAPISchemaEntity(object):
         return "%s '%s'" % (self.meta, self.name)
 
 
-class QAPISchemaVisitor(object):
+class QAPISchemaVisitor:
     def visit_begin(self, schema):
         pass
 
@@ -135,7 +135,7 @@ class QAPISchemaVisitor(object):
         pass
 
 
-class QAPISchemaModule(object):
+class QAPISchemaModule:
     def __init__(self, name):
         self.name = name
         self._entity_list = []
@@ -441,7 +441,7 @@ class QAPISchemaObjectType(QAPISchemaType):
                                        self.features)
 
 
-class QAPISchemaMember(object):
+class QAPISchemaMember:
     """ Represents object members, enum members and features """
     role = 'member'
 
@@ -519,7 +519,7 @@ class QAPISchemaObjectTypeMember(QAPISchemaMember):
                                         self.describe)
 
 
-class QAPISchemaObjectTypeVariants(object):
+class QAPISchemaObjectTypeVariants:
     def __init__(self, tag_name, info, tag_member, variants):
         # Flat unions pass tag_name but not tag_member.
         # Simple unions and alternates pass tag_member but not tag_name.
@@ -787,7 +787,7 @@ class QAPISchemaEvent(QAPISchemaEntity):
                             self.arg_type, self.boxed)
 
 
-class QAPISchema(object):
+class QAPISchema:
     def __init__(self, fname):
         self.fname = fname
         parser = QAPISchemaParser(fname)
diff --git a/scripts/qapi/source.py b/scripts/qapi/source.py
index 8956885033..e97b9a8e15 100644
--- a/scripts/qapi/source.py
+++ b/scripts/qapi/source.py
@@ -13,7 +13,7 @@ import copy
 import sys
 
 
-class QAPISchemaPragma(object):
+class QAPISchemaPragma:
     def __init__(self):
         # Are documentation comments required?
         self.doc_required = False
@@ -23,7 +23,7 @@ class QAPISchemaPragma(object):
         self.name_case_whitelist = []
 
 
-class QAPISourceInfo(object):
+class QAPISourceInfo:
     def __init__(self, fname, line, parent):
         self.fname = fname
         self.line = line
-- 
2.21.1

Re: [PATCH 1/4] qapi: Inheriting from object is pointless with Python 3, drop

[Philippe Mathieu-Daudé]

On 2/27/20 3:45 PM, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   scripts/qapi/gen.py    |  2 +-
>   scripts/qapi/parser.py |  6 +++---
>   scripts/qapi/schema.py | 12 ++++++------
>   scripts/qapi/source.py |  4 ++--
>   4 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
> index 95afae0615..a53a705c73 100644
> --- a/scripts/qapi/gen.py
> +++ b/scripts/qapi/gen.py
> @@ -22,7 +22,7 @@ from qapi.common import *
>   from qapi.schema import QAPISchemaVisitor
>   
>   
> -class QAPIGen(object):
> +class QAPIGen:
>   
>       def __init__(self, fname):
>           self.fname = fname
> diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
> index 342792e410..2e3a3c5d76 100644
> --- a/scripts/qapi/parser.py
> +++ b/scripts/qapi/parser.py
> @@ -23,7 +23,7 @@ from qapi.error import QAPIParseError, QAPISemError
>   from qapi.source import QAPISourceInfo
>   
>   
> -class QAPISchemaParser(object):
> +class QAPISchemaParser:
>   
>       def __init__(self, fname, previously_included=None, incl_info=None):
>           previously_included = previously_included or set()
> @@ -293,7 +293,7 @@ class QAPISchemaParser(object):
>           raise QAPIParseError(self, "documentation comment must end with '##'")
>   
>   
> -class QAPIDoc(object):
> +class QAPIDoc:
>       """
>       A documentation comment block, either definition or free-form
>   
> @@ -312,7 +312,7 @@ class QAPIDoc(object):
>       Free-form documentation blocks consist only of a body section.
>       """
>   
> -    class Section(object):
> +    class Section:
>           def __init__(self, name=None):
>               # optional section name (argument/member or section name)
>               self.name = name
> diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> index 5100110fa2..c8bcfe2c49 100644
> --- a/scripts/qapi/schema.py
> +++ b/scripts/qapi/schema.py
> @@ -24,7 +24,7 @@ from qapi.expr import check_exprs
>   from qapi.parser import QAPISchemaParser
>   
>   
> -class QAPISchemaEntity(object):
> +class QAPISchemaEntity:
>       meta = None
>   
>       def __init__(self, name, info, doc, ifcond=None, features=None):
> @@ -89,7 +89,7 @@ class QAPISchemaEntity(object):
>           return "%s '%s'" % (self.meta, self.name)
>   
>   
> -class QAPISchemaVisitor(object):
> +class QAPISchemaVisitor:
>       def visit_begin(self, schema):
>           pass
>   
> @@ -135,7 +135,7 @@ class QAPISchemaVisitor(object):
>           pass
>   
>   
> -class QAPISchemaModule(object):
> +class QAPISchemaModule:
>       def __init__(self, name):
>           self.name = name
>           self._entity_list = []
> @@ -441,7 +441,7 @@ class QAPISchemaObjectType(QAPISchemaType):
>                                          self.features)
>   
>   
> -class QAPISchemaMember(object):
> +class QAPISchemaMember:
>       """ Represents object members, enum members and features """
>       role = 'member'
>   
> @@ -519,7 +519,7 @@ class QAPISchemaObjectTypeMember(QAPISchemaMember):
>                                           self.describe)
>   
>   
> -class QAPISchemaObjectTypeVariants(object):
> +class QAPISchemaObjectTypeVariants:
>       def __init__(self, tag_name, info, tag_member, variants):
>           # Flat unions pass tag_name but not tag_member.
>           # Simple unions and alternates pass tag_member but not tag_name.
> @@ -787,7 +787,7 @@ class QAPISchemaEvent(QAPISchemaEntity):
>                               self.arg_type, self.boxed)
>   
>   
> -class QAPISchema(object):
> +class QAPISchema:
>       def __init__(self, fname):
>           self.fname = fname
>           parser = QAPISchemaParser(fname)
> diff --git a/scripts/qapi/source.py b/scripts/qapi/source.py
> index 8956885033..e97b9a8e15 100644
> --- a/scripts/qapi/source.py
> +++ b/scripts/qapi/source.py
> @@ -13,7 +13,7 @@ import copy
>   import sys
>   
>   
> -class QAPISchemaPragma(object):
> +class QAPISchemaPragma:
>       def __init__(self):
>           # Are documentation comments required?
>           self.doc_required = False
> @@ -23,7 +23,7 @@ class QAPISchemaPragma(object):
>           self.name_case_whitelist = []
>   
>   
> -class QAPISourceInfo(object):
> +class QAPISourceInfo:
>       def __init__(self, fname, line, parent):
>           self.fname = fname
>           self.line = line
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

[PATCH 2/4] qapi: Drop conditionals for Python 2

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi/common.py         | 6 +-----
 scripts/qapi/gen.py            | 6 +-----
 scripts/qapi/parser.py         | 6 +-----
 tests/qapi-schema/test-qapi.py | 6 +-----
 4 files changed, 4 insertions(+), 20 deletions(-)

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index e00dcafce7..ba35abea47 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -12,7 +12,6 @@
 # See the COPYING file in the top-level directory.
 
 import re
-import string
 
 
 # ENUMName -> ENUM_NAME, EnumName1 -> ENUM_NAME1
@@ -43,10 +42,7 @@ def c_enum_const(type_name, const_name, prefix=None):
     return camel_to_upper(type_name) + '_' + c_name(const_name, False).upper()
 
 
-if hasattr(str, 'maketrans'):
-    c_name_trans = str.maketrans('.-', '__')
-else:
-    c_name_trans = string.maketrans('.-', '__')
+c_name_trans = str.maketrans('.-', '__')
 
 
 # Map @name to a valid C identifier.
diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
index a53a705c73..317cd72601 100644
--- a/scripts/qapi/gen.py
+++ b/scripts/qapi/gen.py
@@ -15,7 +15,6 @@
 import errno
 import os
 import re
-import sys
 from contextlib import contextmanager
 
 from qapi.common import *
@@ -54,10 +53,7 @@ class QAPIGen:
                 if e.errno != errno.EEXIST:
                     raise
         fd = os.open(pathname, os.O_RDWR | os.O_CREAT, 0o666)
-        if sys.version_info[0] >= 3:
-            f = open(fd, 'r+', encoding='utf-8')
-        else:
-            f = os.fdopen(fd, 'r+')
+        f = open(fd, 'r+', encoding='utf-8')
         text = self.get_content()
         oldtext = f.read(len(text) + 1)
         if text != oldtext:
diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index 2e3a3c5d76..cf14e5426c 100644
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -16,7 +16,6 @@
 
 import os
 import re
-import sys
 from collections import OrderedDict
 
 from qapi.error import QAPIParseError, QAPISemError
@@ -30,10 +29,7 @@ class QAPISchemaParser:
         previously_included.add(os.path.abspath(fname))
 
         try:
-            if sys.version_info[0] >= 3:
-                fp = open(fname, 'r', encoding='utf-8')
-            else:
-                fp = open(fname, 'r')
+            fp = open(fname, 'r', encoding='utf-8')
             self.src = fp.read()
         except IOError as e:
             raise QAPISemError(incl_info or QAPISourceInfo(None, None, None),
diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
index 41232c11a3..bee18ee344 100755
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -16,15 +16,11 @@ import argparse
 import difflib
 import os
 import sys
+from io import StringIO
 
 from qapi.error import QAPIError
 from qapi.schema import QAPISchema, QAPISchemaVisitor
 
-if sys.version_info[0] < 3:
-    from cStringIO import StringIO
-else:
-    from io import StringIO
-
 
 class QAPISchemaTestVisitor(QAPISchemaVisitor):
 
-- 
2.21.1

Re: [PATCH 2/4] qapi: Drop conditionals for Python 2

[Philippe Mathieu-Daudé]

On 2/27/20 3:45 PM, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   scripts/qapi/common.py         | 6 +-----
>   scripts/qapi/gen.py            | 6 +-----
>   scripts/qapi/parser.py         | 6 +-----
>   tests/qapi-schema/test-qapi.py | 6 +-----
>   4 files changed, 4 insertions(+), 20 deletions(-)
> 
> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> index e00dcafce7..ba35abea47 100644
> --- a/scripts/qapi/common.py
> +++ b/scripts/qapi/common.py
> @@ -12,7 +12,6 @@
>   # See the COPYING file in the top-level directory.
>   
>   import re
> -import string
>   
>   
>   # ENUMName -> ENUM_NAME, EnumName1 -> ENUM_NAME1
> @@ -43,10 +42,7 @@ def c_enum_const(type_name, const_name, prefix=None):
>       return camel_to_upper(type_name) + '_' + c_name(const_name, False).upper()
>   
>   
> -if hasattr(str, 'maketrans'):
> -    c_name_trans = str.maketrans('.-', '__')
> -else:
> -    c_name_trans = string.maketrans('.-', '__')
> +c_name_trans = str.maketrans('.-', '__')
>   
>   
>   # Map @name to a valid C identifier.
> diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
> index a53a705c73..317cd72601 100644
> --- a/scripts/qapi/gen.py
> +++ b/scripts/qapi/gen.py
> @@ -15,7 +15,6 @@
>   import errno
>   import os
>   import re
> -import sys
>   from contextlib import contextmanager
>   
>   from qapi.common import *
> @@ -54,10 +53,7 @@ class QAPIGen:
>                   if e.errno != errno.EEXIST:
>                       raise
>           fd = os.open(pathname, os.O_RDWR | os.O_CREAT, 0o666)
> -        if sys.version_info[0] >= 3:
> -            f = open(fd, 'r+', encoding='utf-8')
> -        else:
> -            f = os.fdopen(fd, 'r+')
> +        f = open(fd, 'r+', encoding='utf-8')
>           text = self.get_content()
>           oldtext = f.read(len(text) + 1)
>           if text != oldtext:
> diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
> index 2e3a3c5d76..cf14e5426c 100644
> --- a/scripts/qapi/parser.py
> +++ b/scripts/qapi/parser.py
> @@ -16,7 +16,6 @@
>   
>   import os
>   import re
> -import sys
>   from collections import OrderedDict
>   
>   from qapi.error import QAPIParseError, QAPISemError
> @@ -30,10 +29,7 @@ class QAPISchemaParser:
>           previously_included.add(os.path.abspath(fname))
>   
>           try:
> -            if sys.version_info[0] >= 3:
> -                fp = open(fname, 'r', encoding='utf-8')
> -            else:
> -                fp = open(fname, 'r')
> +            fp = open(fname, 'r', encoding='utf-8')
>               self.src = fp.read()
>           except IOError as e:
>               raise QAPISemError(incl_info or QAPISourceInfo(None, None, None),
> diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
> index 41232c11a3..bee18ee344 100755
> --- a/tests/qapi-schema/test-qapi.py
> +++ b/tests/qapi-schema/test-qapi.py
> @@ -16,15 +16,11 @@ import argparse
>   import difflib
>   import os
>   import sys
> +from io import StringIO
>   
>   from qapi.error import QAPIError
>   from qapi.schema import QAPISchema, QAPISchemaVisitor
>   
> -if sys.version_info[0] < 3:
> -    from cStringIO import StringIO
> -else:
> -    from io import StringIO
> -
>   
>   class QAPISchemaTestVisitor(QAPISchemaVisitor):
>   
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

[PATCH 3/4] qapi: Use super() now we have Python 3

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi/commands.py   |  4 +--
 scripts/qapi/error.py      |  4 +--
 scripts/qapi/events.py     |  4 +--
 scripts/qapi/gen.py        | 10 ++++----
 scripts/qapi/introspect.py |  4 +--
 scripts/qapi/parser.py     |  2 +-
 scripts/qapi/schema.py     | 50 +++++++++++++++++++-------------------
 scripts/qapi/types.py      |  4 +--
 scripts/qapi/visit.py      |  4 +--
 9 files changed, 43 insertions(+), 43 deletions(-)

diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
index afa55b055c..8bb6316061 100644
--- a/scripts/qapi/commands.py
+++ b/scripts/qapi/commands.py
@@ -237,8 +237,8 @@ void %(c_prefix)sqmp_init_marshal(QmpCommandList *cmds)
 class QAPISchemaGenCommandVisitor(QAPISchemaModularCVisitor):
 
     def __init__(self, prefix):
-        QAPISchemaModularCVisitor.__init__(
-            self, prefix, 'qapi-commands',
+        super().__init__(
+            prefix, 'qapi-commands',
             ' * Schema-defined QAPI/QMP commands', None, __doc__)
         self._regy = QAPIGenCCode(None)
         self._visited_ret_types = {}
diff --git a/scripts/qapi/error.py b/scripts/qapi/error.py
index b9f3751bea..ae60d9e2fe 100644
--- a/scripts/qapi/error.py
+++ b/scripts/qapi/error.py
@@ -35,9 +35,9 @@ class QAPIParseError(QAPIError):
                 col = (col + 7) % 8 + 1
             else:
                 col += 1
-        QAPIError.__init__(self, parser.info, col, msg)
+        super().__init__(parser.info, col, msg)
 
 
 class QAPISemError(QAPIError):
     def __init__(self, info, msg):
-        QAPIError.__init__(self, info, None, msg)
+        super().__init__(info, None, msg)
diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py
index 2bde3e6128..a98b9f5099 100644
--- a/scripts/qapi/events.py
+++ b/scripts/qapi/events.py
@@ -138,8 +138,8 @@ def gen_event_send(name, arg_type, boxed, event_enum_name, event_emit):
 class QAPISchemaGenEventVisitor(QAPISchemaModularCVisitor):
 
     def __init__(self, prefix):
-        QAPISchemaModularCVisitor.__init__(
-            self, prefix, 'qapi-events',
+        super().__init__(
+            prefix, 'qapi-events',
             ' * Schema-defined QAPI/QMP events', None, __doc__)
         self._event_enum_name = c_name(prefix + 'QAPIEvent', protect=False)
         self._event_enum_members = []
diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
index 317cd72601..e17354392b 100644
--- a/scripts/qapi/gen.py
+++ b/scripts/qapi/gen.py
@@ -82,7 +82,7 @@ def _wrap_ifcond(ifcond, before, after):
 class QAPIGenCCode(QAPIGen):
 
     def __init__(self, fname):
-        QAPIGen.__init__(self, fname)
+        super().__init__(fname)
         self._start_if = None
 
     def start_if(self, ifcond):
@@ -102,13 +102,13 @@ class QAPIGenCCode(QAPIGen):
 
     def get_content(self):
         assert self._start_if is None
-        return QAPIGen.get_content(self)
+        return super().get_content()
 
 
 class QAPIGenC(QAPIGenCCode):
 
     def __init__(self, fname, blurb, pydoc):
-        QAPIGenCCode.__init__(self, fname)
+        super().__init__(fname)
         self._blurb = blurb
         self._copyright = '\n * '.join(re.findall(r'^Copyright .*', pydoc,
                                                   re.MULTILINE))
@@ -141,7 +141,7 @@ char qapi_dummy_%(name)s;
 class QAPIGenH(QAPIGenC):
 
     def _top(self):
-        return QAPIGenC._top(self) + guardstart(self.fname)
+        return super()._top() + guardstart(self.fname)
 
     def _bottom(self):
         return guardend(self.fname)
@@ -176,7 +176,7 @@ def ifcontext(ifcond, *args):
 class QAPIGenDoc(QAPIGen):
 
     def _top(self):
-        return (QAPIGen._top(self)
+        return (super()._top()
                 + '@c AUTOMATICALLY GENERATED, DO NOT MODIFY\n\n')
 
 
diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index b3a463dd8b..0cc655fd9f 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -76,8 +76,8 @@ def to_c_string(string):
 class QAPISchemaGenIntrospectVisitor(QAPISchemaMonolithicCVisitor):
 
     def __init__(self, prefix, unmask):
-        QAPISchemaMonolithicCVisitor.__init__(
-            self, prefix, 'qapi-introspect',
+        super().__init__(
+            prefix, 'qapi-introspect',
             ' * QAPI/QMP schema introspection', __doc__)
         self._unmask = unmask
         self._schema = None
diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index cf14e5426c..340f7c4633 100644
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -320,7 +320,7 @@ class QAPIDoc:
 
     class ArgSection(Section):
         def __init__(self, name):
-            QAPIDoc.Section.__init__(self, name)
+            super().__init__(name)
             self.member = None
 
         def connect(self, member):
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index c8bcfe2c49..e132442c04 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -152,11 +152,11 @@ class QAPISchemaModule:
 
 class QAPISchemaInclude(QAPISchemaEntity):
     def __init__(self, sub_module, info):
-        QAPISchemaEntity.__init__(self, None, info, None)
+        super().__init__(None, info, None)
         self._sub_module = sub_module
 
     def visit(self, visitor):
-        QAPISchemaEntity.visit(self, visitor)
+        super().visit(visitor)
         visitor.visit_include(self._sub_module.name, self.info)
 
 
@@ -202,7 +202,7 @@ class QAPISchemaBuiltinType(QAPISchemaType):
     meta = 'built-in'
 
     def __init__(self, name, json_type, c_type):
-        QAPISchemaType.__init__(self, name, None, None)
+        super().__init__(name, None, None)
         assert not c_type or isinstance(c_type, str)
         assert json_type in ('string', 'number', 'int', 'boolean', 'null',
                              'value')
@@ -227,7 +227,7 @@ class QAPISchemaBuiltinType(QAPISchemaType):
         return self.json_type()
 
     def visit(self, visitor):
-        QAPISchemaType.visit(self, visitor)
+        super().visit(visitor)
         visitor.visit_builtin_type(self.name, self.info, self.json_type())
 
 
@@ -235,7 +235,7 @@ class QAPISchemaEnumType(QAPISchemaType):
     meta = 'enum'
 
     def __init__(self, name, info, doc, ifcond, members, prefix):
-        QAPISchemaType.__init__(self, name, info, doc, ifcond)
+        super().__init__(name, info, doc, ifcond)
         for m in members:
             assert isinstance(m, QAPISchemaEnumMember)
             m.set_defined_in(name)
@@ -244,7 +244,7 @@ class QAPISchemaEnumType(QAPISchemaType):
         self.prefix = prefix
 
     def check(self, schema):
-        QAPISchemaType.check(self, schema)
+        super().check(schema)
         seen = {}
         for m in self.members:
             m.check_clash(self.info, seen)
@@ -269,7 +269,7 @@ class QAPISchemaEnumType(QAPISchemaType):
         return 'string'
 
     def visit(self, visitor):
-        QAPISchemaType.visit(self, visitor)
+        super().visit(visitor)
         visitor.visit_enum_type(self.name, self.info, self.ifcond,
                                 self.members, self.prefix)
 
@@ -278,13 +278,13 @@ class QAPISchemaArrayType(QAPISchemaType):
     meta = 'array'
 
     def __init__(self, name, info, element_type):
-        QAPISchemaType.__init__(self, name, info, None, None)
+        super().__init__(name, info, None, None)
         assert isinstance(element_type, str)
         self._element_type_name = element_type
         self.element_type = None
 
     def check(self, schema):
-        QAPISchemaType.check(self, schema)
+        super().check(schema)
         self.element_type = schema.resolve_type(
             self._element_type_name, self.info,
             self.info and self.info.defn_meta)
@@ -314,7 +314,7 @@ class QAPISchemaArrayType(QAPISchemaType):
         return 'array of ' + elt_doc_type
 
     def visit(self, visitor):
-        QAPISchemaType.visit(self, visitor)
+        super().visit(visitor)
         visitor.visit_array_type(self.name, self.info, self.ifcond,
                                  self.element_type)
 
@@ -329,7 +329,7 @@ class QAPISchemaObjectType(QAPISchemaType):
         # struct has local_members, optional base, and no variants
         # flat union has base, variants, and no local_members
         # simple union has local_members, variants, and no base
-        QAPISchemaType.__init__(self, name, info, doc, ifcond, features)
+        super().__init__(name, info, doc, ifcond, features)
         self.meta = 'union' if variants else 'struct'
         assert base is None or isinstance(base, str)
         for m in local_members:
@@ -356,7 +356,7 @@ class QAPISchemaObjectType(QAPISchemaType):
             raise QAPISemError(self.info,
                                "object %s contains itself" % self.name)
 
-        QAPISchemaType.check(self, schema)
+        super().check(schema)
         assert self._checked and self.members is None
 
         seen = OrderedDict()
@@ -419,7 +419,7 @@ class QAPISchemaObjectType(QAPISchemaType):
 
     def c_name(self):
         assert self.name != 'q_empty'
-        return QAPISchemaType.c_name(self)
+        return super().c_name()
 
     def c_type(self):
         assert not self.is_implicit()
@@ -432,7 +432,7 @@ class QAPISchemaObjectType(QAPISchemaType):
         return 'object'
 
     def visit(self, visitor):
-        QAPISchemaType.visit(self, visitor)
+        super().visit(visitor)
         visitor.visit_object_type(self.name, self.info, self.ifcond,
                                   self.base, self.local_members, self.variants,
                                   self.features)
@@ -506,7 +506,7 @@ class QAPISchemaFeature(QAPISchemaMember):
 
 class QAPISchemaObjectTypeMember(QAPISchemaMember):
     def __init__(self, name, info, typ, optional, ifcond=None):
-        QAPISchemaMember.__init__(self, name, info, ifcond)
+        super().__init__(name, info, ifcond)
         assert isinstance(typ, str)
         assert isinstance(optional, bool)
         self._type_name = typ
@@ -614,7 +614,7 @@ class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember):
     role = 'branch'
 
     def __init__(self, name, info, typ, ifcond=None):
-        QAPISchemaObjectTypeMember.__init__(self, name, info, typ,
+        super().__init__(name, info, typ,
                                             False, ifcond)
 
 
@@ -622,7 +622,7 @@ class QAPISchemaAlternateType(QAPISchemaType):
     meta = 'alternate'
 
     def __init__(self, name, info, doc, ifcond, variants):
-        QAPISchemaType.__init__(self, name, info, doc, ifcond)
+        super().__init__(name, info, doc, ifcond)
         assert isinstance(variants, QAPISchemaObjectTypeVariants)
         assert variants.tag_member
         variants.set_defined_in(name)
@@ -630,7 +630,7 @@ class QAPISchemaAlternateType(QAPISchemaType):
         self.variants = variants
 
     def check(self, schema):
-        QAPISchemaType.check(self, schema)
+        super().check(schema)
         self.variants.tag_member.check(schema)
         # Not calling self.variants.check_clash(), because there's nothing
         # to clash with
@@ -680,7 +680,7 @@ class QAPISchemaAlternateType(QAPISchemaType):
         return 'value'
 
     def visit(self, visitor):
-        QAPISchemaType.visit(self, visitor)
+        super().visit(visitor)
         visitor.visit_alternate_type(self.name, self.info, self.ifcond,
                                      self.variants)
 
@@ -691,7 +691,7 @@ class QAPISchemaCommand(QAPISchemaEntity):
     def __init__(self, name, info, doc, ifcond, arg_type, ret_type,
                  gen, success_response, boxed, allow_oob, allow_preconfig,
                  features):
-        QAPISchemaEntity.__init__(self, name, info, doc, ifcond, features)
+        super().__init__(name, info, doc, ifcond, features)
         assert not arg_type or isinstance(arg_type, str)
         assert not ret_type or isinstance(ret_type, str)
         self._arg_type_name = arg_type
@@ -705,7 +705,7 @@ class QAPISchemaCommand(QAPISchemaEntity):
         self.allow_preconfig = allow_preconfig
 
     def check(self, schema):
-        QAPISchemaEntity.check(self, schema)
+        super().check(schema)
         if self._arg_type_name:
             self.arg_type = schema.resolve_type(
                 self._arg_type_name, self.info, "command's 'data'")
@@ -740,7 +740,7 @@ class QAPISchemaCommand(QAPISchemaEntity):
                 self.arg_type.connect_doc(doc)
 
     def visit(self, visitor):
-        QAPISchemaEntity.visit(self, visitor)
+        super().visit(visitor)
         visitor.visit_command(self.name, self.info, self.ifcond,
                               self.arg_type, self.ret_type,
                               self.gen, self.success_response,
@@ -753,14 +753,14 @@ class QAPISchemaEvent(QAPISchemaEntity):
     meta = 'event'
 
     def __init__(self, name, info, doc, ifcond, arg_type, boxed):
-        QAPISchemaEntity.__init__(self, name, info, doc, ifcond)
+        super().__init__(name, info, doc, ifcond)
         assert not arg_type or isinstance(arg_type, str)
         self._arg_type_name = arg_type
         self.arg_type = None
         self.boxed = boxed
 
     def check(self, schema):
-        QAPISchemaEntity.check(self, schema)
+        super().check(schema)
         if self._arg_type_name:
             self.arg_type = schema.resolve_type(
                 self._arg_type_name, self.info, "event's 'data'")
@@ -782,7 +782,7 @@ class QAPISchemaEvent(QAPISchemaEntity):
                 self.arg_type.connect_doc(doc)
 
     def visit(self, visitor):
-        QAPISchemaEntity.visit(self, visitor)
+        super().visit(visitor)
         visitor.visit_event(self.name, self.info, self.ifcond,
                             self.arg_type, self.boxed)
 
diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
index 99dcaf7074..3c83b6e4be 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -241,8 +241,8 @@ void qapi_free_%(c_name)s(%(c_name)s *obj)
 class QAPISchemaGenTypeVisitor(QAPISchemaModularCVisitor):
 
     def __init__(self, prefix):
-        QAPISchemaModularCVisitor.__init__(
-            self, prefix, 'qapi-types', ' * Schema-defined QAPI types',
+        super().__init__(
+            prefix, 'qapi-types', ' * Schema-defined QAPI types',
             ' * Built-in QAPI types', __doc__)
 
     def _begin_system_module(self, name):
diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
index 4efce62b0c..421e5bd8cd 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -283,8 +283,8 @@ out:
 class QAPISchemaGenVisitVisitor(QAPISchemaModularCVisitor):
 
     def __init__(self, prefix):
-        QAPISchemaModularCVisitor.__init__(
-            self, prefix, 'qapi-visit', ' * Schema-defined QAPI visitors',
+        super().__init__(
+            prefix, 'qapi-visit', ' * Schema-defined QAPI visitors',
             ' * Built-in QAPI visitors', __doc__)
 
     def _begin_system_module(self, name):
-- 
2.21.1

Re: [PATCH 3/4] qapi: Use super() now we have Python 3

[Markus Armbruster]

Markus Armbruster <armbru@redhat.com> writes:

> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
[...]
> diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> index c8bcfe2c49..e132442c04 100644
> --- a/scripts/qapi/schema.py
> +++ b/scripts/qapi/schema.py
[...]
> @@ -614,7 +614,7 @@ class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember):
>      role = 'branch'
>  
>      def __init__(self, name, info, typ, ifcond=None):
> -        QAPISchemaObjectTypeMember.__init__(self, name, info, typ,
> +        super().__init__(name, info, typ,
>                                              False, ifcond)

pycodestyle-3 gripes:

    scripts/qapi/schema.py:618:45: E127 continuation line over-indented for visual indent

Will fix.

[...]

[PATCH 4/4] qapi: Brush off some (py)lint

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi/commands.py   | 2 +-
 scripts/qapi/expr.py       | 3 +--
 scripts/qapi/gen.py        | 9 ++++++---
 scripts/qapi/introspect.py | 2 --
 scripts/qapi/parser.py     | 6 ++----
 scripts/qapi/schema.py     | 9 ++++-----
 6 files changed, 14 insertions(+), 17 deletions(-)

diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
index 8bb6316061..0e13e82989 100644
--- a/scripts/qapi/commands.py
+++ b/scripts/qapi/commands.py
@@ -274,7 +274,7 @@ class QAPISchemaGenCommandVisitor(QAPISchemaModularCVisitor):
 
 void %(c_prefix)sqmp_init_marshal(QmpCommandList *cmds);
 ''',
-                       c_prefix=c_name(self._prefix, protect=False)))
+                             c_prefix=c_name(self._prefix, protect=False)))
         self._genc.preamble_add(mcgen('''
 #include "qemu/osdep.h"
 #include "%(prefix)sqapi-commands.h"
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index d7a289eded..fecf466fa7 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -35,7 +35,6 @@ def check_name_is_str(name, info, source):
 def check_name_str(name, info, source,
                    allow_optional=False, enum_member=False,
                    permit_upper=False):
-    global valid_name
     membername = name
 
     if allow_optional and name.startswith('*'):
@@ -249,7 +248,7 @@ def check_union(expr, info):
 def check_alternate(expr, info):
     members = expr['data']
 
-    if len(members) == 0:
+    if not members:
         raise QAPISemError(info, "'data' must not be empty")
     for (key, value) in members.items():
         source = "'data' member '%s'" % key
diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
index e17354392b..33690bfa3b 100644
--- a/scripts/qapi/gen.py
+++ b/scripts/qapi/gen.py
@@ -45,10 +45,10 @@ class QAPIGen:
 
     def write(self, output_dir):
         pathname = os.path.join(output_dir, self.fname)
-        dir = os.path.dirname(pathname)
-        if dir:
+        odir = os.path.dirname(pathname)
+        if odir:
             try:
-                os.makedirs(dir)
+                os.makedirs(odir)
             except os.error as e:
                 if e.errno != errno.EEXIST:
                     raise
@@ -261,6 +261,9 @@ class QAPISchemaModularCVisitor(QAPISchemaVisitor):
             genc.write(output_dir)
             genh.write(output_dir)
 
+    def _begin_system_module(self, name):
+        pass
+
     def _begin_user_module(self, name):
         pass
 
diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index 0cc655fd9f..b5537eddc0 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -10,8 +10,6 @@ This work is licensed under the terms of the GNU GPL, version 2.
 See the COPYING file in the top-level directory.
 """
 
-import string
-
 from qapi.common import *
 from qapi.gen import QAPISchemaMonolithicCVisitor
 from qapi.schema import (QAPISchemaArrayType, QAPISchemaBuiltinType,
diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index 340f7c4633..abadacbb0e 100644
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -282,8 +282,7 @@ class QAPISchemaParser:
                 doc.end_comment()
                 self.accept()
                 return doc
-            else:
-                doc.append(self.val)
+            doc.append(self.val)
             self.accept(False)
 
         raise QAPIParseError(self, "documentation comment must end with '##'")
@@ -492,7 +491,7 @@ class QAPIDoc:
             raise QAPIParseError(self._parser,
                                  "'%s' can't follow '%s' section"
                                  % (name, self.sections[0].name))
-        elif self._is_section_tag(name):
+        if self._is_section_tag(name):
             line = line[len(name)+1:]
             self._start_section(name[:-1])
 
@@ -556,7 +555,6 @@ class QAPIDoc:
             raise QAPISemError(feature.info,
                                "feature '%s' lacks documentation"
                                % feature.name)
-            self.features[feature.name] = QAPIDoc.ArgSection(feature.name)
         self.features[feature.name].connect(feature)
 
     def check_expr(self, expr):
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index e132442c04..cfbb9758c4 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -19,7 +19,7 @@ import re
 from collections import OrderedDict
 
 from qapi.common import c_name, pointer_suffix
-from qapi.error import QAPIError, QAPIParseError, QAPISemError
+from qapi.error import QAPIError, QAPISemError
 from qapi.expr import check_exprs
 from qapi.parser import QAPISchemaParser
 
@@ -96,14 +96,14 @@ class QAPISchemaVisitor:
     def visit_end(self):
         pass
 
-    def visit_module(self, fname):
+    def visit_module(self, name):
         pass
 
     def visit_needed(self, entity):
         # Default to visiting everything
         return True
 
-    def visit_include(self, fname, info):
+    def visit_include(self, name, info):
         pass
 
     def visit_builtin_type(self, name, info, json_type):
@@ -576,7 +576,7 @@ class QAPISchemaObjectTypeVariants:
             assert self.tag_member.ifcond == []
         if self._tag_name:    # flat union
             # branches that are not explicitly covered get an empty type
-            cases = set([v.name for v in self.variants])
+            cases = {v.name for v in self.variants}
             for m in self.tag_member.type.members:
                 if m.name not in cases:
                     v = QAPISchemaObjectTypeVariant(m.name, self.info,
@@ -1098,7 +1098,6 @@ class QAPISchema:
 
     def visit(self, visitor):
         visitor.visit_begin(self)
-        module = None
         for mod in self._module_dict.values():
             mod.visit(visitor)
         visitor.visit_end()
-- 
2.21.1

Re: [PATCH 4/4] qapi: Brush off some (py)lint

[John Snow]

On 2/27/20 9:45 AM, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

I wrote some pylint cleanup for iotests recently, too. Are you targeting
a subset of pylint errors to clean here?

(Do any files pass 100%?)

Consider checking in a pylintrc file that lets others run the same
subset of pylint tests as you are doing so that we can prevent future
regressions.

Take a peek at [PATCH v6 0/9] iotests: use python logging​

Thanks for this series. I had a very similar series sitting waiting to
go out, but this goes further in a few places.

--js

> ---
>  scripts/qapi/commands.py   | 2 +-
>  scripts/qapi/expr.py       | 3 +--
>  scripts/qapi/gen.py        | 9 ++++++---
>  scripts/qapi/introspect.py | 2 --
>  scripts/qapi/parser.py     | 6 ++----
>  scripts/qapi/schema.py     | 9 ++++-----
>  6 files changed, 14 insertions(+), 17 deletions(-)
> 
> diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
> index 8bb6316061..0e13e82989 100644
> --- a/scripts/qapi/commands.py
> +++ b/scripts/qapi/commands.py
> @@ -274,7 +274,7 @@ class QAPISchemaGenCommandVisitor(QAPISchemaModularCVisitor):
>  
>  void %(c_prefix)sqmp_init_marshal(QmpCommandList *cmds);
>  ''',
> -                       c_prefix=c_name(self._prefix, protect=False)))
> +                             c_prefix=c_name(self._prefix, protect=False)))
>          self._genc.preamble_add(mcgen('''
>  #include "qemu/osdep.h"
>  #include "%(prefix)sqapi-commands.h"
> diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
> index d7a289eded..fecf466fa7 100644
> --- a/scripts/qapi/expr.py
> +++ b/scripts/qapi/expr.py
> @@ -35,7 +35,6 @@ def check_name_is_str(name, info, source):
>  def check_name_str(name, info, source,
>                     allow_optional=False, enum_member=False,
>                     permit_upper=False):
> -    global valid_name
>      membername = name
>  
>      if allow_optional and name.startswith('*'):
> @@ -249,7 +248,7 @@ def check_union(expr, info):
>  def check_alternate(expr, info):
>      members = expr['data']
>  
> -    if len(members) == 0:
> +    if not members:
>          raise QAPISemError(info, "'data' must not be empty")
>      for (key, value) in members.items():
>          source = "'data' member '%s'" % key
> diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
> index e17354392b..33690bfa3b 100644
> --- a/scripts/qapi/gen.py
> +++ b/scripts/qapi/gen.py
> @@ -45,10 +45,10 @@ class QAPIGen:
>  
>      def write(self, output_dir):
>          pathname = os.path.join(output_dir, self.fname)
> -        dir = os.path.dirname(pathname)
> -        if dir:
> +        odir = os.path.dirname(pathname)
> +        if odir:
>              try:
> -                os.makedirs(dir)
> +                os.makedirs(odir)
>              except os.error as e:
>                  if e.errno != errno.EEXIST:
>                      raise
> @@ -261,6 +261,9 @@ class QAPISchemaModularCVisitor(QAPISchemaVisitor):
>              genc.write(output_dir)
>              genh.write(output_dir)
>  
> +    def _begin_system_module(self, name):
> +        pass
> +
>      def _begin_user_module(self, name):
>          pass
>  
> diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
> index 0cc655fd9f..b5537eddc0 100644
> --- a/scripts/qapi/introspect.py
> +++ b/scripts/qapi/introspect.py
> @@ -10,8 +10,6 @@ This work is licensed under the terms of the GNU GPL, version 2.
>  See the COPYING file in the top-level directory.
>  """
>  
> -import string
> -
>  from qapi.common import *
>  from qapi.gen import QAPISchemaMonolithicCVisitor
>  from qapi.schema import (QAPISchemaArrayType, QAPISchemaBuiltinType,
> diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
> index 340f7c4633..abadacbb0e 100644
> --- a/scripts/qapi/parser.py
> +++ b/scripts/qapi/parser.py
> @@ -282,8 +282,7 @@ class QAPISchemaParser:
>                  doc.end_comment()
>                  self.accept()
>                  return doc
> -            else:
> -                doc.append(self.val)
> +            doc.append(self.val)
>              self.accept(False)
>  
>          raise QAPIParseError(self, "documentation comment must end with '##'")
> @@ -492,7 +491,7 @@ class QAPIDoc:
>              raise QAPIParseError(self._parser,
>                                   "'%s' can't follow '%s' section"
>                                   % (name, self.sections[0].name))
> -        elif self._is_section_tag(name):
> +        if self._is_section_tag(name):
>              line = line[len(name)+1:]
>              self._start_section(name[:-1])
>  
> @@ -556,7 +555,6 @@ class QAPIDoc:
>              raise QAPISemError(feature.info,
>                                 "feature '%s' lacks documentation"
>                                 % feature.name)
> -            self.features[feature.name] = QAPIDoc.ArgSection(feature.name)
>          self.features[feature.name].connect(feature)
>  
>      def check_expr(self, expr):
> diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> index e132442c04..cfbb9758c4 100644
> --- a/scripts/qapi/schema.py
> +++ b/scripts/qapi/schema.py
> @@ -19,7 +19,7 @@ import re
>  from collections import OrderedDict
>  
>  from qapi.common import c_name, pointer_suffix
> -from qapi.error import QAPIError, QAPIParseError, QAPISemError
> +from qapi.error import QAPIError, QAPISemError
>  from qapi.expr import check_exprs
>  from qapi.parser import QAPISchemaParser
>  
> @@ -96,14 +96,14 @@ class QAPISchemaVisitor:
>      def visit_end(self):
>          pass
>  
> -    def visit_module(self, fname):
> +    def visit_module(self, name):
>          pass
>  
>      def visit_needed(self, entity):
>          # Default to visiting everything
>          return True
>  
> -    def visit_include(self, fname, info):
> +    def visit_include(self, name, info):
>          pass
>  
>      def visit_builtin_type(self, name, info, json_type):
> @@ -576,7 +576,7 @@ class QAPISchemaObjectTypeVariants:
>              assert self.tag_member.ifcond == []
>          if self._tag_name:    # flat union
>              # branches that are not explicitly covered get an empty type
> -            cases = set([v.name for v in self.variants])
> +            cases = {v.name for v in self.variants}
>              for m in self.tag_member.type.members:
>                  if m.name not in cases:
>                      v = QAPISchemaObjectTypeVariant(m.name, self.info,
> @@ -1098,7 +1098,6 @@ class QAPISchema:
>  
>      def visit(self, visitor):
>          visitor.visit_begin(self)
> -        module = None
>          for mod in self._module_dict.values():
>              mod.visit(visitor)
>          visitor.visit_end()
> 

-- 
—js

Re: [PATCH 4/4] qapi: Brush off some (py)lint

[Markus Armbruster]

John Snow <jsnow@redhat.com> writes:

> On 2/27/20 9:45 AM, Markus Armbruster wrote:
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>
> I wrote some pylint cleanup for iotests recently, too. Are you targeting
> a subset of pylint errors to clean here?
>
> (Do any files pass 100%?)

Surely you're joking, Mr. Snow!

I'm chipping away at pylint's gripes.  I ran it with the following
messages disabled:

    bad-whitespace,
    fixme,
    invalid-name,
    missing-docstring,
    too-few-public-methods,
    too-many-arguments,
    too-many-branches,
    too-many-instance-attributes,
    too-many-lines,
    too-many-locals,
    too-many-statements,
    unused-argument,
    unused-wildcard-import,

These are not all obviously useless.  They're just not what I want to
focus on right now.

Remaining:

1 x C0330: Wrong continued indentation (remove 19 spaces).

    Accident, will fix in v2.

8 x R0201: Method could be a function (no-self-use)

    Yes, but the override in a sub-class does use self.

2 x W0212: Access to a protected member _body of a client class (protected-access)

    Needs cleanup, but not now.

6 x W0401: Wildcard import qapi.common (wildcard-import)

    Not sure I care.  I'd prefer not to have more wildcard imports,
    though.

2 x W0603: Using the global statement (global-statement)

    Cleanup is non-trivial.  Not now.

I also ran pycodestyle-3:

1 x E127 continuation line over-indented for visual indent

    Same as pylint's C0330, will fix in v2.

3 x E261 at least two spaces before inline comment

    I blame Emacs.  Left for another day.

8 x E501 line too long

    Left for another day.

1 x E713 test for membership should be 'not in'

    I missed that one, will fix in v2.

> Consider checking in a pylintrc file that lets others run the same
> subset of pylint tests as you are doing so that we can prevent future
> regressions.

Working towards it, slowly.

> Take a peek at [PATCH v6 0/9] iotests: use python logging​
>
> Thanks for this series. I had a very similar series sitting waiting to
> go out, but this goes further in a few places.

Thanks!

Re: [PATCH 4/4] qapi: Brush off some (py)lint

[John Snow]

On 3/4/20 3:01 AM, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:
> 
>> On 2/27/20 9:45 AM, Markus Armbruster wrote:
>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>
>> I wrote some pylint cleanup for iotests recently, too. Are you targeting
>> a subset of pylint errors to clean here?
>>
>> (Do any files pass 100%?)
> 
> Surely you're joking, Mr. Snow!
> 
> I'm chipping away at pylint's gripes.  I ran it with the following
> messages disabled:
> 
>     bad-whitespace,
>     fixme,
>     invalid-name,
>     missing-docstring,
>     too-few-public-methods,
>     too-many-arguments,
>     too-many-branches,
>     too-many-instance-attributes,
>     too-many-lines,
>     too-many-locals,
>     too-many-statements,
>     unused-argument,
>     unused-wildcard-import,
> 
> These are not all obviously useless.  They're just not what I want to
> focus on right now.
> 

Yes, understood - so my approach is disable what I don't intend to fix,
commit the pylintrc to prevent backslide, and move on.

I think we have a difference in what a pylintrc means to us (the goal
vs. the current status.)

I didn't mean "100% without caveats", just "100% in some subset of checks".

(I assume the answer is still no.)

> Remaining:
> 
> 1 x C0330: Wrong continued indentation (remove 19 spaces).
> 
>     Accident, will fix in v2.
> 
> 8 x R0201: Method could be a function (no-self-use)
> 
>     Yes, but the override in a sub-class does use self.
> 
> 2 x W0212: Access to a protected member _body of a client class (protected-access)
> 
>     Needs cleanup, but not now.
> 
> 6 x W0401: Wildcard import qapi.common (wildcard-import)
> 
>     Not sure I care.  I'd prefer not to have more wildcard imports,
>     though.
> 
> 2 x W0603: Using the global statement (global-statement)
> 
>     Cleanup is non-trivial.  Not now.
> 
> I also ran pycodestyle-3:
> 
> 1 x E127 continuation line over-indented for visual indent
> 
>     Same as pylint's C0330, will fix in v2.
> 
> 3 x E261 at least two spaces before inline comment
> 
>     I blame Emacs.  Left for another day.
> 
> 8 x E501 line too long
> 
>     Left for another day.
> 
> 1 x E713 test for membership should be 'not in'
> 
>     I missed that one, will fix in v2.
> 
>> Consider checking in a pylintrc file that lets others run the same
>> subset of pylint tests as you are doing so that we can prevent future
>> regressions.
> 
> Working towards it, slowly.
> 
>> Take a peek at [PATCH v6 0/9] iotests: use python logging​
>>
>> Thanks for this series. I had a very similar series sitting waiting to
>> go out, but this goes further in a few places.
> 
> Thanks!
> 

Re: [PATCH 4/4] qapi: Brush off some (py)lint

[Markus Armbruster]

John Snow <jsnow@redhat.com> writes:

> On 3/4/20 3:01 AM, Markus Armbruster wrote:
>> John Snow <jsnow@redhat.com> writes:
>> 
>>> On 2/27/20 9:45 AM, Markus Armbruster wrote:
>>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>>
>>> I wrote some pylint cleanup for iotests recently, too. Are you targeting
>>> a subset of pylint errors to clean here?
>>>
>>> (Do any files pass 100%?)
>> 
>> Surely you're joking, Mr. Snow!
>> 
>> I'm chipping away at pylint's gripes.  I ran it with the following
>> messages disabled:
>> 
>>     bad-whitespace,
>>     fixme,
>>     invalid-name,
>>     missing-docstring,
>>     too-few-public-methods,
>>     too-many-arguments,
>>     too-many-branches,
>>     too-many-instance-attributes,
>>     too-many-lines,
>>     too-many-locals,
>>     too-many-statements,
>>     unused-argument,
>>     unused-wildcard-import,
>> 
>> These are not all obviously useless.  They're just not what I want to
>> focus on right now.
>> 
>
> Yes, understood - so my approach is disable what I don't intend to fix,
> commit the pylintrc to prevent backslide, and move on.
>
> I think we have a difference in what a pylintrc means to us (the goal
> vs. the current status.)
>
> I didn't mean "100% without caveats", just "100% in some subset of checks".
>
> (I assume the answer is still no.)

To turn the answer into a yes, I'd have to disable the messages below,
and some of them I'd rather keep.

Tacking

    # pylint: disable=...

to existing troublemakers may or may not be worth the ugliness (it needs
to go on the same line, which almost invariably makes it awkwardly long).

>> Remaining:
>> 
>> 1 x C0330: Wrong continued indentation (remove 19 spaces).
>> 
>>     Accident, will fix in v2.
>> 
>> 8 x R0201: Method could be a function (no-self-use)
>> 
>>     Yes, but the override in a sub-class does use self.
>> 
>> 2 x W0212: Access to a protected member _body of a client class (protected-access)
>> 
>>     Needs cleanup, but not now.
>> 
>> 6 x W0401: Wildcard import qapi.common (wildcard-import)
>> 
>>     Not sure I care.  I'd prefer not to have more wildcard imports,
>>     though.
>> 
>> 2 x W0603: Using the global statement (global-statement)
>> 
>>     Cleanup is non-trivial.  Not now.
>> 
>> I also ran pycodestyle-3:
>> 
>> 1 x E127 continuation line over-indented for visual indent
>> 
>>     Same as pylint's C0330, will fix in v2.
>> 
>> 3 x E261 at least two spaces before inline comment
>> 
>>     I blame Emacs.  Left for another day.
>> 
>> 8 x E501 line too long
>> 
>>     Left for another day.
>> 
>> 1 x E713 test for membership should be 'not in'
>> 
>>     I missed that one, will fix in v2.
>> 
>>> Consider checking in a pylintrc file that lets others run the same
>>> subset of pylint tests as you are doing so that we can prevent future
>>> regressions.
>> 
>> Working towards it, slowly.
>> 
>>> Take a peek at [PATCH v6 0/9] iotests: use python logging​
>>>
>>> Thanks for this series. I had a very similar series sitting waiting to
>>> go out, but this goes further in a few places.
>> 
>> Thanks!
>>