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
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
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!
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! >
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!
>>
© 2016 - 2025 Red Hat, Inc.