| Message ID | 20210216021809.134886-20-jsnow@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | qapi: static typing conversion, pt2 | expand |
John Snow <jsnow@redhat.com> writes: > Follows the qapi/introspect.py definition of the same; this adds a more > precise typing to _gen_tree's mtype parameter. > > NB: print(SchemaMetaType.BUILTIN) would produce the string > "SchemaMetaType.BUILTIN", but when using format strings (.format or f-strings), > it relies on the __format__ method defined in the Enum class, which uses the > "value" of the enum instead, producing the string "builtin". > > For consistency with old-style format strings (which simply call the > __str__ method of an object), a __str__ dunder is added, though it is > not actually used here in this code. > > Signed-off-by: John Snow <jsnow@redhat.com> > --- > scripts/qapi/introspect.py | 38 +++++++++++++++++++++++++++++--------- > 1 file changed, 29 insertions(+), 9 deletions(-) > > diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py > index c6f5cf8d874..008a21f5c4c 100644 > --- a/scripts/qapi/introspect.py > +++ b/scripts/qapi/introspect.py > @@ -11,6 +11,7 @@ > See the COPYING file in the top-level directory. > """ > > +from enum import Enum > from typing import ( > Any, > Dict, > @@ -79,6 +80,23 @@ > SchemaInfoCommand = Dict[str, object] > > > +class SchemaMetaType(str, Enum): > + """ > + Mimics the SchemaMetaType enum from qapi/introspect.json. > + """ > + BUILTIN = 'builtin' > + ENUM = 'enum' > + ARRAY = 'array' > + OBJECT = 'object' > + ALTERNATE = 'alternate' > + COMMAND = 'command' > + EVENT = 'event' > + > + def __str__(self) -> str: > + # Needed for intuitive behavior with old-style format strings. > + return str(self.value) > + > + The fanciness compared to plain Enum('SchemaMetaType', 'BUILTIN ...') avoids extra code to map the enum values to the strings with need. > _ValueT = TypeVar('_ValueT', bound=_Value) > > > @@ -251,7 +269,8 @@ 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], > + def _gen_tree(self, name: str, mtype: SchemaMetaType, > + obj: Dict[str, object], > ifcond: Sequence[str] = (), > features: Sequence[QAPISchemaFeature] = ()) -> None: > """ > @@ -299,7 +318,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}) > + self._gen_tree(name, SchemaMetaType.BUILTIN, {'json-type': json_type}) > > def visit_enum_type(self, name: str, info: Optional[QAPISourceInfo], > ifcond: Sequence[str], > @@ -307,7 +326,7 @@ def visit_enum_type(self, name: str, info: Optional[QAPISourceInfo], > members: List[QAPISchemaEnumMember], > prefix: Optional[str]) -> None: > self._gen_tree( > - name, 'enum', > + name, SchemaMetaType.ENUM, > {'values': [Annotated(m.name, m.ifcond) for m in members]}, > ifcond, features > ) > @@ -316,8 +335,8 @@ def visit_array_type(self, name: str, info: Optional[QAPISourceInfo], > ifcond: Sequence[str], > element_type: QAPISchemaType) -> None: > element = self._use_type(element_type) > - self._gen_tree('[' + element + ']', 'array', {'element-type': element}, > - ifcond) > + self._gen_tree('[' + element + ']', SchemaMetaType.ARRAY, > + {'element-type': element}, ifcond) > > def visit_object_type_flat(self, name: str, info: Optional[QAPISourceInfo], > ifcond: Sequence[str], > @@ -330,14 +349,14 @@ def visit_object_type_flat(self, name: str, info: Optional[QAPISourceInfo], > if variants: > obj['tag'] = variants.tag_member.name > obj['variants'] = [self._gen_variant(v) for v in variants.variants] > - self._gen_tree(name, 'object', obj, ifcond, features) > + self._gen_tree(name, SchemaMetaType.OBJECT, obj, ifcond, features) > > def visit_alternate_type(self, name: str, info: Optional[QAPISourceInfo], > ifcond: Sequence[str], > features: List[QAPISchemaFeature], > variants: QAPISchemaVariants) -> None: > self._gen_tree( > - name, 'alternate', > + name, SchemaMetaType.ALTERNATE, > {'members': [Annotated({'type': self._use_type(m.type)}, > m.ifcond) > for m in variants.variants]}, > @@ -361,7 +380,7 @@ def visit_command(self, name: str, info: Optional[QAPISourceInfo], > } > if allow_oob: > obj['allow-oob'] = allow_oob > - self._gen_tree(name, 'command', obj, ifcond, features) > + self._gen_tree(name, SchemaMetaType.COMMAND, obj, ifcond, features) > > def visit_event(self, name: str, info: Optional[QAPISourceInfo], > ifcond: Sequence[str], features: List[QAPISchemaFeature], > @@ -370,7 +389,8 @@ def visit_event(self, name: str, info: Optional[QAPISourceInfo], > assert self._schema is not None > > arg_type = arg_type or self._schema.the_empty_object_type > - self._gen_tree(name, 'event', {'arg-type': self._use_type(arg_type)}, > + self._gen_tree(name, SchemaMetaType.EVENT, > + {'arg-type': self._use_type(arg_type)}, > ifcond, features) Gain: _gen_tree()'s second argument's type now serves as documentation, and passing crap to it becomes harder. Gut feeling: too much notational overhead for too little gain. Opinions?
On 2/16/21 4:24 AM, Markus Armbruster wrote: > John Snow <jsnow@redhat.com> writes: > >> Follows the qapi/introspect.py definition of the same; this adds a more >> precise typing to _gen_tree's mtype parameter. >> >> NB: print(SchemaMetaType.BUILTIN) would produce the string >> "SchemaMetaType.BUILTIN", but when using format strings (.format or f-strings), >> it relies on the __format__ method defined in the Enum class, which uses the >> "value" of the enum instead, producing the string "builtin". >> >> For consistency with old-style format strings (which simply call the >> __str__ method of an object), a __str__ dunder is added, though it is >> not actually used here in this code. >> >> Signed-off-by: John Snow <jsnow@redhat.com> >> --- >> scripts/qapi/introspect.py | 38 +++++++++++++++++++++++++++++--------- >> 1 file changed, 29 insertions(+), 9 deletions(-) >> >> diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py >> index c6f5cf8d874..008a21f5c4c 100644 >> --- a/scripts/qapi/introspect.py >> +++ b/scripts/qapi/introspect.py >> @@ -11,6 +11,7 @@ >> See the COPYING file in the top-level directory. >> """ >> >> +from enum import Enum >> from typing import ( >> Any, >> Dict, >> @@ -79,6 +80,23 @@ >> SchemaInfoCommand = Dict[str, object] >> >> >> +class SchemaMetaType(str, Enum): >> + """ >> + Mimics the SchemaMetaType enum from qapi/introspect.json. >> + """ >> + BUILTIN = 'builtin' >> + ENUM = 'enum' >> + ARRAY = 'array' >> + OBJECT = 'object' >> + ALTERNATE = 'alternate' >> + COMMAND = 'command' >> + EVENT = 'event' >> + >> + def __str__(self) -> str: >> + # Needed for intuitive behavior with old-style format strings. >> + return str(self.value) >> + >> + > > The fanciness compared to plain Enum('SchemaMetaType', 'BUILTIN ...') > avoids extra code to map the enum values to the strings with need. > I wasn't even aware there was a short form. (TIL!) This form allows me to inherit from str and pass the value around anywhere strings are used. Due to the generalized nature of tree_to_qlit, using the short constructor form (which creates ints) would need additional magic to be useful. You can almost replicate it: _values = ('builtin', 'enum', 'array') _nv_pairs = [(value.upper(), value) for value in _values] SchemaMetaType = enum.Enum('SchemaMetaType', _nv_pairs, type=str) though this loses out on the __str__ method hack, and I don't think mypy will be able to introspect into this functional constructor. >> _ValueT = TypeVar('_ValueT', bound=_Value) >> >> >> @@ -251,7 +269,8 @@ 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], >> + def _gen_tree(self, name: str, mtype: SchemaMetaType, >> + obj: Dict[str, object], >> ifcond: Sequence[str] = (), >> features: Sequence[QAPISchemaFeature] = ()) -> None: >> """ >> @@ -299,7 +318,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}) >> + self._gen_tree(name, SchemaMetaType.BUILTIN, {'json-type': json_type}) >> >> def visit_enum_type(self, name: str, info: Optional[QAPISourceInfo], >> ifcond: Sequence[str], >> @@ -307,7 +326,7 @@ def visit_enum_type(self, name: str, info: Optional[QAPISourceInfo], >> members: List[QAPISchemaEnumMember], >> prefix: Optional[str]) -> None: >> self._gen_tree( >> - name, 'enum', >> + name, SchemaMetaType.ENUM, >> {'values': [Annotated(m.name, m.ifcond) for m in members]}, >> ifcond, features >> ) >> @@ -316,8 +335,8 @@ def visit_array_type(self, name: str, info: Optional[QAPISourceInfo], >> ifcond: Sequence[str], >> element_type: QAPISchemaType) -> None: >> element = self._use_type(element_type) >> - self._gen_tree('[' + element + ']', 'array', {'element-type': element}, >> - ifcond) >> + self._gen_tree('[' + element + ']', SchemaMetaType.ARRAY, >> + {'element-type': element}, ifcond) >> >> def visit_object_type_flat(self, name: str, info: Optional[QAPISourceInfo], >> ifcond: Sequence[str], >> @@ -330,14 +349,14 @@ def visit_object_type_flat(self, name: str, info: Optional[QAPISourceInfo], >> if variants: >> obj['tag'] = variants.tag_member.name >> obj['variants'] = [self._gen_variant(v) for v in variants.variants] >> - self._gen_tree(name, 'object', obj, ifcond, features) >> + self._gen_tree(name, SchemaMetaType.OBJECT, obj, ifcond, features) >> >> def visit_alternate_type(self, name: str, info: Optional[QAPISourceInfo], >> ifcond: Sequence[str], >> features: List[QAPISchemaFeature], >> variants: QAPISchemaVariants) -> None: >> self._gen_tree( >> - name, 'alternate', >> + name, SchemaMetaType.ALTERNATE, >> {'members': [Annotated({'type': self._use_type(m.type)}, >> m.ifcond) >> for m in variants.variants]}, >> @@ -361,7 +380,7 @@ def visit_command(self, name: str, info: Optional[QAPISourceInfo], >> } >> if allow_oob: >> obj['allow-oob'] = allow_oob >> - self._gen_tree(name, 'command', obj, ifcond, features) >> + self._gen_tree(name, SchemaMetaType.COMMAND, obj, ifcond, features) >> >> def visit_event(self, name: str, info: Optional[QAPISourceInfo], >> ifcond: Sequence[str], features: List[QAPISchemaFeature], >> @@ -370,7 +389,8 @@ def visit_event(self, name: str, info: Optional[QAPISourceInfo], >> assert self._schema is not None >> >> arg_type = arg_type or self._schema.the_empty_object_type >> - self._gen_tree(name, 'event', {'arg-type': self._use_type(arg_type)}, >> + self._gen_tree(name, SchemaMetaType.EVENT, >> + {'arg-type': self._use_type(arg_type)}, >> ifcond, features) > > Gain: _gen_tree()'s second argument's type now serves as documentation, > and passing crap to it becomes harder. > > Gut feeling: too much notational overhead for too little gain. > > Opinions? > No strong feelings one way or the other, honestly. I wrote this mostly to see how much the overhead would be based on your comment about the loose typing of meta-type as str. If it's too much for too little for you, I'm okay without it too. --js
John Snow <jsnow@redhat.com> writes: > On 2/16/21 4:24 AM, Markus Armbruster wrote: >> John Snow <jsnow@redhat.com> writes: >> >>> Follows the qapi/introspect.py definition of the same; this adds a more >>> precise typing to _gen_tree's mtype parameter. >>> >>> NB: print(SchemaMetaType.BUILTIN) would produce the string >>> "SchemaMetaType.BUILTIN", but when using format strings (.format or f-strings), >>> it relies on the __format__ method defined in the Enum class, which uses the >>> "value" of the enum instead, producing the string "builtin". >>> >>> For consistency with old-style format strings (which simply call the >>> __str__ method of an object), a __str__ dunder is added, though it is >>> not actually used here in this code. >>> >>> Signed-off-by: John Snow <jsnow@redhat.com> [...] >> Gain: _gen_tree()'s second argument's type now serves as documentation, >> and passing crap to it becomes harder. >> >> Gut feeling: too much notational overhead for too little gain. >> >> Opinions? >> > > No strong feelings one way or the other, honestly. I wrote this mostly > to see how much the overhead would be based on your comment about the > loose typing of meta-type as str. > > If it's too much for too little for you, I'm okay without it too. Let's put it aside for now.
diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py index c6f5cf8d874..008a21f5c4c 100644 --- a/scripts/qapi/introspect.py +++ b/scripts/qapi/introspect.py @@ -11,6 +11,7 @@ See the COPYING file in the top-level directory. """ +from enum import Enum from typing import ( Any, Dict, @@ -79,6 +80,23 @@ SchemaInfoCommand = Dict[str, object] +class SchemaMetaType(str, Enum): + """ + Mimics the SchemaMetaType enum from qapi/introspect.json. + """ + BUILTIN = 'builtin' + ENUM = 'enum' + ARRAY = 'array' + OBJECT = 'object' + ALTERNATE = 'alternate' + COMMAND = 'command' + EVENT = 'event' + + def __str__(self) -> str: + # Needed for intuitive behavior with old-style format strings. + return str(self.value) + + _ValueT = TypeVar('_ValueT', bound=_Value) @@ -251,7 +269,8 @@ 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], + def _gen_tree(self, name: str, mtype: SchemaMetaType, + obj: Dict[str, object], ifcond: Sequence[str] = (), features: Sequence[QAPISchemaFeature] = ()) -> None: """ @@ -299,7 +318,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}) + self._gen_tree(name, SchemaMetaType.BUILTIN, {'json-type': json_type}) def visit_enum_type(self, name: str, info: Optional[QAPISourceInfo], ifcond: Sequence[str], @@ -307,7 +326,7 @@ def visit_enum_type(self, name: str, info: Optional[QAPISourceInfo], members: List[QAPISchemaEnumMember], prefix: Optional[str]) -> None: self._gen_tree( - name, 'enum', + name, SchemaMetaType.ENUM, {'values': [Annotated(m.name, m.ifcond) for m in members]}, ifcond, features ) @@ -316,8 +335,8 @@ def visit_array_type(self, name: str, info: Optional[QAPISourceInfo], ifcond: Sequence[str], element_type: QAPISchemaType) -> None: element = self._use_type(element_type) - self._gen_tree('[' + element + ']', 'array', {'element-type': element}, - ifcond) + self._gen_tree('[' + element + ']', SchemaMetaType.ARRAY, + {'element-type': element}, ifcond) def visit_object_type_flat(self, name: str, info: Optional[QAPISourceInfo], ifcond: Sequence[str], @@ -330,14 +349,14 @@ def visit_object_type_flat(self, name: str, info: Optional[QAPISourceInfo], if variants: obj['tag'] = variants.tag_member.name obj['variants'] = [self._gen_variant(v) for v in variants.variants] - self._gen_tree(name, 'object', obj, ifcond, features) + self._gen_tree(name, SchemaMetaType.OBJECT, obj, ifcond, features) def visit_alternate_type(self, name: str, info: Optional[QAPISourceInfo], ifcond: Sequence[str], features: List[QAPISchemaFeature], variants: QAPISchemaVariants) -> None: self._gen_tree( - name, 'alternate', + name, SchemaMetaType.ALTERNATE, {'members': [Annotated({'type': self._use_type(m.type)}, m.ifcond) for m in variants.variants]}, @@ -361,7 +380,7 @@ def visit_command(self, name: str, info: Optional[QAPISourceInfo], } if allow_oob: obj['allow-oob'] = allow_oob - self._gen_tree(name, 'command', obj, ifcond, features) + self._gen_tree(name, SchemaMetaType.COMMAND, obj, ifcond, features) def visit_event(self, name: str, info: Optional[QAPISourceInfo], ifcond: Sequence[str], features: List[QAPISchemaFeature], @@ -370,7 +389,8 @@ def visit_event(self, name: str, info: Optional[QAPISourceInfo], assert self._schema is not None arg_type = arg_type or self._schema.the_empty_object_type - self._gen_tree(name, 'event', {'arg-type': self._use_type(arg_type)}, + self._gen_tree(name, SchemaMetaType.EVENT, + {'arg-type': self._use_type(arg_type)}, ifcond, features)
Follows the qapi/introspect.py definition of the same; this adds a more precise typing to _gen_tree's mtype parameter. NB: print(SchemaMetaType.BUILTIN) would produce the string "SchemaMetaType.BUILTIN", but when using format strings (.format or f-strings), it relies on the __format__ method defined in the Enum class, which uses the "value" of the enum instead, producing the string "builtin". For consistency with old-style format strings (which simply call the __str__ method of an object), a __str__ dunder is added, though it is not actually used here in this code. Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/introspect.py | 38 +++++++++++++++++++++++++++++--------- 1 file changed, 29 insertions(+), 9 deletions(-)