qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v6 17/19] qapi/introspect.py: Type _gen_tree variants as Sequ


From: John Snow
Subject: Re: [PATCH v6 17/19] qapi/introspect.py: Type _gen_tree variants as Sequence[str]
Date: Tue, 16 Feb 2021 10:13:42 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0

On 2/16/21 4:10 AM, Markus Armbruster wrote:
John Snow <jsnow@redhat.com> writes:

Optional[List] is clunky; an empty sequence can more elegantly convey
"no variants". By downgrading "List" to "Sequence", we can also accept
tuples; this is useful for the empty tuple specifically, which we may
use as a default parameter because it is immutable.

Signed-off-by: John Snow <jsnow@redhat.com>
---
  scripts/qapi/introspect.py | 11 ++++++-----
  1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index 649225988d1..e4d31a59503 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -247,13 +247,13 @@ def _use_type(self, typ: QAPISchemaType) -> str:
          return self._name(typ.name)
@staticmethod
-    def _gen_features(features: List[QAPISchemaFeature]
+    def _gen_features(features: Sequence[QAPISchemaFeature]
                        ) -> List[Annotated[str]]:
          return [Annotated(f.name, f.ifcond) for f in features]
def _gen_tree(self, name: str, mtype: str, obj: Dict[str, object],
                    ifcond: Sequence[str],
-                  features: Optional[List[QAPISchemaFeature]]) -> None:
+                  features: Sequence[QAPISchemaFeature] = ()) -> None:
          """
          Build and append a SchemaInfo object to self._trees.
@@ -261,7 +261,8 @@ def _gen_tree(self, name: str, mtype: str, obj: Dict[str, object],
          :param mtype: The entity's meta-type.
          :param obj: Additional entity fields, as appropriate for the 
meta-type.
          :param ifcond: Sequence of conditionals that apply to this entity.
-        :param features: Optional features field for SchemaInfo.
+        :param features: Optional, The features field for SchemaInfo.

Downcase "The".

+                         Will be omitted from the output if empty.
          """
          comment: Optional[str] = None
          if mtype not in ('command', 'event', 'builtin', 'array'):
@@ -298,7 +299,7 @@ def _gen_variant(self, variant: QAPISchemaVariant
def visit_builtin_type(self, name: str, info: Optional[QAPISourceInfo],
                             json_type: str) -> None:
-        self._gen_tree(name, 'builtin', {'json-type': json_type}, [], None)
+        self._gen_tree(name, 'builtin', {'json-type': json_type}, [])
def visit_enum_type(self, name: str, info: Optional[QAPISourceInfo],
                          ifcond: Sequence[str],
@@ -316,7 +317,7 @@ def visit_array_type(self, name: str, info: 
Optional[QAPISourceInfo],
                           element_type: QAPISchemaType) -> None:
          element = self._use_type(element_type)
          self._gen_tree('[' + element + ']', 'array', {'element-type': 
element},
-                       ifcond, None)
+                       ifcond)
def visit_object_type_flat(self, name: str, info: Optional[QAPISourceInfo],
                                 ifcond: Sequence[str],

Marginal by itself.  Might be worth it as part of a more general move
away from Optional[List[...]].  See also next patch.


Yep, it's tiny. Still, maybe worth taking just to remove a bad habit from the code in case of cargo-culting?

Go with whatcha feel, it's a style nit you raised -- I'm not sure I'll remember to do a more thorough pass after pt6 is done.

ACK to change the comment casing if you take these.

--js




reply via email to

[Prev in Thread] Current Thread [Next in Thread]