| Message ID | 20200925002900.465855-13-jsnow@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | qapi: static typing conversion, pt1 | expand |
John Snow <jsnow@redhat.com> writes: > While we're mucking around with imports, we might as well formalize the > style we use. Let's use isort to do it for us. > > force_sort_within_sections: Intermingles "from x" and "import x" style > statements, such that sorting is always performed strictly on the module > name itself. > > force_grid_wrap=4: Four or more imports from a single module will force > the one-per-line style that's more git-friendly. This will generally > happen for 'typing' imports. > > multi_line_output=3: Uses the one-per-line indented style for long > imports. > > include_trailing_comma: Adds a comma to the last import in a group, > which makes git conflicts nicer to deal with, generally. > > Suggested-by: Cleber Rosa <crosa@redhat.com> > Signed-off-by: John Snow <jsnow@redhat.com> > --- > scripts/qapi/.isort.cfg | 5 +++++ > scripts/qapi/commands.py | 6 +----- > scripts/qapi/doc.py | 2 +- > scripts/qapi/expr.py | 4 ++-- > scripts/qapi/introspect.py | 3 +-- > scripts/qapi/main.py | 1 - > scripts/qapi/parser.py | 2 +- > scripts/qapi/schema.py | 2 +- > scripts/qapi/types.py | 1 - > 9 files changed, 12 insertions(+), 14 deletions(-) > create mode 100644 scripts/qapi/.isort.cfg > > diff --git a/scripts/qapi/.isort.cfg b/scripts/qapi/.isort.cfg > new file mode 100644 > index 0000000000..b0aeffec26 > --- /dev/null > +++ b/scripts/qapi/.isort.cfg > @@ -0,0 +1,5 @@ > +[settings] > +force_sort_within_sections=True > +force_grid_wrap=4 > +multi_line_output=3 > +include_trailing_comma=True > diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py > index e1df0e341f..64ed5278f9 100644 > --- a/scripts/qapi/commands.py > +++ b/scripts/qapi/commands.py > @@ -13,11 +13,7 @@ > See the COPYING file in the top-level directory. > """ > > -from .common import ( > - build_params, > - c_name, > - mcgen, > -) > +from .common import build_params, c_name, mcgen > from .gen import QAPIGenCCode, QAPISchemaModularCVisitor, ifcontext Squash this hunk into the previous commit, please. > > > diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py > index b764a8ccc0..1acb773e0a 100644 > --- a/scripts/qapi/doc.py > +++ b/scripts/qapi/doc.py > @@ -5,9 +5,9 @@ > """This script produces the documentation of a qapi schema in texinfo format""" > > import re > + > from .gen import QAPIGenDoc, QAPISchemaVisitor > > - Do you delete the blank line to keep isort happy? More of the same below. > _MSG = ''' > @deftypefn {type} {{}} {name} > > diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py > index 03b31ecfc1..3e952a1462 100644 > --- a/scripts/qapi/expr.py > +++ b/scripts/qapi/expr.py > @@ -14,12 +14,12 @@ > # This work is licensed under the terms of the GNU GPL, version 2. > # See the COPYING file in the top-level directory. > > -import re > from collections import OrderedDict > +import re > + > from .common import c_name > from .error import QAPISemError > > - > # Names must be letters, numbers, -, and _. They must start with letter, > # except for downstream extensions which must start with __RFQDN_. > # Dots are only valid in the downstream extension prefix. > diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py > index b036fcf9ce..2850121cbd 100644 > --- a/scripts/qapi/introspect.py > +++ b/scripts/qapi/introspect.py > @@ -17,8 +17,7 @@ > mcgen, > ) > from .gen import QAPISchemaMonolithicCVisitor > -from .schema import (QAPISchemaArrayType, QAPISchemaBuiltinType, > - QAPISchemaType) > +from .schema import QAPISchemaArrayType, QAPISchemaBuiltinType, QAPISchemaType Line is rather long for my taste. So far, we didn't even try to have a consistent Python style in QEMU, so I've enforced a consistent style I like within QAPI. Having a consistent Python style in QEMU would be worth letting go of personal preferences. Do you intend to pursue it? [...]
On 9/25/20 5:20 AM, Markus Armbruster wrote: > John Snow <jsnow@redhat.com> writes: > >> While we're mucking around with imports, we might as well formalize the >> style we use. Let's use isort to do it for us. >> >> force_sort_within_sections: Intermingles "from x" and "import x" style >> statements, such that sorting is always performed strictly on the module >> name itself. >> >> force_grid_wrap=4: Four or more imports from a single module will force >> the one-per-line style that's more git-friendly. This will generally >> happen for 'typing' imports. >> >> multi_line_output=3: Uses the one-per-line indented style for long >> imports. >> >> include_trailing_comma: Adds a comma to the last import in a group, >> which makes git conflicts nicer to deal with, generally. >> >> Suggested-by: Cleber Rosa <crosa@redhat.com> >> Signed-off-by: John Snow <jsnow@redhat.com> >> --- >> scripts/qapi/.isort.cfg | 5 +++++ >> scripts/qapi/commands.py | 6 +----- >> scripts/qapi/doc.py | 2 +- >> scripts/qapi/expr.py | 4 ++-- >> scripts/qapi/introspect.py | 3 +-- >> scripts/qapi/main.py | 1 - >> scripts/qapi/parser.py | 2 +- >> scripts/qapi/schema.py | 2 +- >> scripts/qapi/types.py | 1 - >> 9 files changed, 12 insertions(+), 14 deletions(-) >> create mode 100644 scripts/qapi/.isort.cfg >> >> diff --git a/scripts/qapi/.isort.cfg b/scripts/qapi/.isort.cfg >> new file mode 100644 >> index 0000000000..b0aeffec26 >> --- /dev/null >> +++ b/scripts/qapi/.isort.cfg >> @@ -0,0 +1,5 @@ >> +[settings] >> +force_sort_within_sections=True >> +force_grid_wrap=4 >> +multi_line_output=3 >> +include_trailing_comma=True >> diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py >> index e1df0e341f..64ed5278f9 100644 >> --- a/scripts/qapi/commands.py >> +++ b/scripts/qapi/commands.py >> @@ -13,11 +13,7 @@ >> See the COPYING file in the top-level directory. >> """ >> >> -from .common import ( >> - build_params, >> - c_name, >> - mcgen, >> -) >> +from .common import build_params, c_name, mcgen >> from .gen import QAPIGenCCode, QAPISchemaModularCVisitor, ifcontext > > Squash this hunk into the previous commit, please. > Yes, OK. >> >> >> diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py >> index b764a8ccc0..1acb773e0a 100644 >> --- a/scripts/qapi/doc.py >> +++ b/scripts/qapi/doc.py >> @@ -5,9 +5,9 @@ >> """This script produces the documentation of a qapi schema in texinfo format""" >> >> import re >> + >> from .gen import QAPIGenDoc, QAPISchemaVisitor >> >> - > > Do you delete the blank line to keep isort happy? > isort offers to actually edit your files for you, it isn't just a checker, it actually rewrites. By default, it leaves one space below imports. You can configure it to add two. I didn't really have a strong preference, so I left it at the default. I'm going to leave it at the default for now, but I can change it if you have a preference. > More of the same below. > >> _MSG = ''' >> @deftypefn {type} {{}} {name} >> >> diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py >> index 03b31ecfc1..3e952a1462 100644 >> --- a/scripts/qapi/expr.py >> +++ b/scripts/qapi/expr.py >> @@ -14,12 +14,12 @@ >> # This work is licensed under the terms of the GNU GPL, version 2. >> # See the COPYING file in the top-level directory. >> >> -import re >> from collections import OrderedDict >> +import re >> + >> from .common import c_name >> from .error import QAPISemError >> >> - >> # Names must be letters, numbers, -, and _. They must start with letter, >> # except for downstream extensions which must start with __RFQDN_. >> # Dots are only valid in the downstream extension prefix. >> diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py >> index b036fcf9ce..2850121cbd 100644 >> --- a/scripts/qapi/introspect.py >> +++ b/scripts/qapi/introspect.py >> @@ -17,8 +17,7 @@ >> mcgen, >> ) >> from .gen import QAPISchemaMonolithicCVisitor >> -from .schema import (QAPISchemaArrayType, QAPISchemaBuiltinType, >> - QAPISchemaType) >> +from .schema import QAPISchemaArrayType, QAPISchemaBuiltinType, QAPISchemaType > > Line is rather long for my taste. > 78 chars and three names, so it didn't trigger the grid-wrap condition of isort. If you have an arbitrarily smaller number you'd like to use, I can feed that to isort's config file. 72? > So far, we didn't even try to have a consistent Python style in QEMU, so > I've enforced a consistent style I like within QAPI. > I am now trying! > Having a consistent Python style in QEMU would be worth letting go of > personal preferences. Do you intend to pursue it? > 100%. --js
John Snow <jsnow@redhat.com> writes: > On 9/25/20 5:20 AM, Markus Armbruster wrote: >> John Snow <jsnow@redhat.com> writes: >> >>> While we're mucking around with imports, we might as well formalize the >>> style we use. Let's use isort to do it for us. >>> >>> force_sort_within_sections: Intermingles "from x" and "import x" style >>> statements, such that sorting is always performed strictly on the module >>> name itself. >>> >>> force_grid_wrap=4: Four or more imports from a single module will force >>> the one-per-line style that's more git-friendly. This will generally >>> happen for 'typing' imports. >>> >>> multi_line_output=3: Uses the one-per-line indented style for long >>> imports. >>> >>> include_trailing_comma: Adds a comma to the last import in a group, >>> which makes git conflicts nicer to deal with, generally. >>> >>> Suggested-by: Cleber Rosa <crosa@redhat.com> >>> Signed-off-by: John Snow <jsnow@redhat.com> >>> --- >>> scripts/qapi/.isort.cfg | 5 +++++ >>> scripts/qapi/commands.py | 6 +----- >>> scripts/qapi/doc.py | 2 +- >>> scripts/qapi/expr.py | 4 ++-- >>> scripts/qapi/introspect.py | 3 +-- >>> scripts/qapi/main.py | 1 - >>> scripts/qapi/parser.py | 2 +- >>> scripts/qapi/schema.py | 2 +- >>> scripts/qapi/types.py | 1 - >>> 9 files changed, 12 insertions(+), 14 deletions(-) >>> create mode 100644 scripts/qapi/.isort.cfg >>> >>> diff --git a/scripts/qapi/.isort.cfg b/scripts/qapi/.isort.cfg >>> new file mode 100644 >>> index 0000000000..b0aeffec26 >>> --- /dev/null >>> +++ b/scripts/qapi/.isort.cfg >>> @@ -0,0 +1,5 @@ >>> +[settings] >>> +force_sort_within_sections=True >>> +force_grid_wrap=4 >>> +multi_line_output=3 >>> +include_trailing_comma=True >>> diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py >>> index e1df0e341f..64ed5278f9 100644 >>> --- a/scripts/qapi/commands.py >>> +++ b/scripts/qapi/commands.py >>> @@ -13,11 +13,7 @@ >>> See the COPYING file in the top-level directory. >>> """ >>> -from .common import ( >>> - build_params, >>> - c_name, >>> - mcgen, >>> -) >>> +from .common import build_params, c_name, mcgen >>> from .gen import QAPIGenCCode, QAPISchemaModularCVisitor, ifcontext >> Squash this hunk into the previous commit, please. >> > > Yes, OK. > >>> >>> diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py >>> index b764a8ccc0..1acb773e0a 100644 >>> --- a/scripts/qapi/doc.py >>> +++ b/scripts/qapi/doc.py >>> @@ -5,9 +5,9 @@ >>> """This script produces the documentation of a qapi schema in texinfo format""" >>> import re >>> + >>> from .gen import QAPIGenDoc, QAPISchemaVisitor >>> - >> Do you delete the blank line to keep isort happy? >> > > isort offers to actually edit your files for you, it isn't just a > checker, it actually rewrites. > > By default, it leaves one space below imports. You can configure it to > add two. I didn't really have a strong preference, so I left it at the > default. > > I'm going to leave it at the default for now, but I can change it if > you have a preference. PEP 8: "Surround top-level function and class definitions with two blank lines." [...]
On 9/28/20 8:13 AM, Markus Armbruster wrote: > PEP 8: "Surround top-level function and class definitions with two blank > lines." > > [...] > > Yep, but flake8 does not complain about the first definitions that occur below imports. Why not? I don't know. Regardless, I can change it and fold the changes in; they won't affect much here. --js
On Mon, Sep 28, 2020 at 10:34:42AM -0400, John Snow wrote: > On 9/28/20 8:13 AM, Markus Armbruster wrote: > > PEP 8: "Surround top-level function and class definitions with two blank > > lines." > > > > [...] > > > > > > Yep, but flake8 does not complain about the first definitions that occur > below imports. Why not? I don't know. > > Regardless, I can change it and fold the changes in; they won't affect much > here. > Well, on all of the cases here, it's not function or class definition that follows, so that aspect of PEP 8 is not being violated. - Cleber.
On Thu, Sep 24, 2020 at 08:28:25PM -0400, John Snow wrote: > While we're mucking around with imports, we might as well formalize the > style we use. Let's use isort to do it for us. > > force_sort_within_sections: Intermingles "from x" and "import x" style > statements, such that sorting is always performed strictly on the module > name itself. > > force_grid_wrap=4: Four or more imports from a single module will force > the one-per-line style that's more git-friendly. This will generally > happen for 'typing' imports. > > multi_line_output=3: Uses the one-per-line indented style for long > imports. > > include_trailing_comma: Adds a comma to the last import in a group, > which makes git conflicts nicer to deal with, generally. > > Suggested-by: Cleber Rosa <crosa@redhat.com> > Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Cleber Rosa <crosa@redhat.com> Tested-by: Cleber Rosa <crosa@redhat.com>
diff --git a/scripts/qapi/.isort.cfg b/scripts/qapi/.isort.cfg new file mode 100644 index 0000000000..b0aeffec26 --- /dev/null +++ b/scripts/qapi/.isort.cfg @@ -0,0 +1,5 @@ +[settings] +force_sort_within_sections=True +force_grid_wrap=4 +multi_line_output=3 +include_trailing_comma=True diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py index e1df0e341f..64ed5278f9 100644 --- a/scripts/qapi/commands.py +++ b/scripts/qapi/commands.py @@ -13,11 +13,7 @@ See the COPYING file in the top-level directory. """ -from .common import ( - build_params, - c_name, - mcgen, -) +from .common import build_params, c_name, mcgen from .gen import QAPIGenCCode, QAPISchemaModularCVisitor, ifcontext diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py index b764a8ccc0..1acb773e0a 100644 --- a/scripts/qapi/doc.py +++ b/scripts/qapi/doc.py @@ -5,9 +5,9 @@ """This script produces the documentation of a qapi schema in texinfo format""" import re + from .gen import QAPIGenDoc, QAPISchemaVisitor - _MSG = ''' @deftypefn {type} {{}} {name} diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py index 03b31ecfc1..3e952a1462 100644 --- a/scripts/qapi/expr.py +++ b/scripts/qapi/expr.py @@ -14,12 +14,12 @@ # This work is licensed under the terms of the GNU GPL, version 2. # See the COPYING file in the top-level directory. -import re from collections import OrderedDict +import re + from .common import c_name from .error import QAPISemError - # Names must be letters, numbers, -, and _. They must start with letter, # except for downstream extensions which must start with __RFQDN_. # Dots are only valid in the downstream extension prefix. diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py index b036fcf9ce..2850121cbd 100644 --- a/scripts/qapi/introspect.py +++ b/scripts/qapi/introspect.py @@ -17,8 +17,7 @@ mcgen, ) from .gen import QAPISchemaMonolithicCVisitor -from .schema import (QAPISchemaArrayType, QAPISchemaBuiltinType, - QAPISchemaType) +from .schema import QAPISchemaArrayType, QAPISchemaBuiltinType, QAPISchemaType def _make_tree(obj, ifcond, features, extra=None): diff --git a/scripts/qapi/main.py b/scripts/qapi/main.py index 3f8338ade8..b2f20581fd 100644 --- a/scripts/qapi/main.py +++ b/scripts/qapi/main.py @@ -20,7 +20,6 @@ from .types import gen_types from .visit import gen_visit - DEFAULT_OUTPUT_DIR = '' DEFAULT_PREFIX = '' diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index 76d28c1ce9..fd89e2188b 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -14,9 +14,9 @@ # This work is licensed under the terms of the GNU GPL, version 2. # See the COPYING file in the top-level directory. +from collections import OrderedDict import os import re -from collections import OrderedDict from .error import QAPIParseError, QAPISemError from .source import QAPISourceInfo diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index a835ee6fde..093f7a38d8 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -14,9 +14,9 @@ # TODO catching name collisions in generated code would be nice +from collections import OrderedDict import os import re -from collections import OrderedDict from .common import c_name, pointer_suffix from .error import QAPIError, QAPISemError diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py index 53b47f9e58..cc6dad4c89 100644 --- a/scripts/qapi/types.py +++ b/scripts/qapi/types.py @@ -23,7 +23,6 @@ from .gen import QAPISchemaModularCVisitor, ifcontext from .schema import QAPISchemaEnumMember, QAPISchemaObjectType - # variants must be emitted before their container; track what has already # been output objects_seen = set()
While we're mucking around with imports, we might as well formalize the style we use. Let's use isort to do it for us. force_sort_within_sections: Intermingles "from x" and "import x" style statements, such that sorting is always performed strictly on the module name itself. force_grid_wrap=4: Four or more imports from a single module will force the one-per-line style that's more git-friendly. This will generally happen for 'typing' imports. multi_line_output=3: Uses the one-per-line indented style for long imports. include_trailing_comma: Adds a comma to the last import in a group, which makes git conflicts nicer to deal with, generally. Suggested-by: Cleber Rosa <crosa@redhat.com> Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/.isort.cfg | 5 +++++ scripts/qapi/commands.py | 6 +----- scripts/qapi/doc.py | 2 +- scripts/qapi/expr.py | 4 ++-- scripts/qapi/introspect.py | 3 +-- scripts/qapi/main.py | 1 - scripts/qapi/parser.py | 2 +- scripts/qapi/schema.py | 2 +- scripts/qapi/types.py | 1 - 9 files changed, 12 insertions(+), 14 deletions(-) create mode 100644 scripts/qapi/.isort.cfg