Add some aliases that declare intent for some of the "dictly-typed"
objects we pass around in introspect.py.
Signed-off-by: John Snow <jsnow@redhat.com>
---
This patch is optional, it can be dropped if desired. If it's taken,
it's probably best to squash it with the prior patch. It is purely for
the sake of demonstrating what the _DObject was meant to convey: a
Python Dict that represents some JSON Object. It does not add any type
safety in and of itself, but does have some (light) annotational
benefit. In this case, it's usually a specific data structure from the
QAPI Schema we are referencing, but do not have "compile-time" access to
in Python.
These are loosely typed and don't bother reproducing the exact structure
of the real types. Python 3.6 does not have support for TypedDict
structures, so this is as good as we can do without involving a
third-party library (e.g. Pydantic), in which we might be able to say:
class SchemaMetaType(str, enum.Enum):
BUILTIN = "builtin"
ENUM = "enum"
ARRAY = "array"
OBJECT = "object"
ALTERNATE = "alternate"
COMMAND = "command"
EVENT = "event"
class SchemaInfo(pydantic.BaseModel):
name: str
meta-type: SchemaMetaType
features: Optional[List[str]]
data: Union[SchemaInfoBuiltin, SchemaInfoEnum, SchemaInfoArray,
SchemaInfoObject, SchemaInfoAlternate, SchemaInfoCommand,
SchemaInfoEvent]
However, the cost of reproducing and keeping these structure definitions
in-sync between the user-defined portion of the schema and the code
generator is likely not worth doing any such thing. However, it does
illustrate an interesting dependency the generator has on the
user-defined schema itself in terms of types.
So, I settled on using some light types that suggest the form of the
object instead of enforcing the form.
Signed-off-by: John Snow <jsnow@redhat.com>
---
scripts/qapi/introspect.py | 51 +++++++++++++++++++++++---------------
1 file changed, 31 insertions(+), 20 deletions(-)
diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index 3afcdda7446..2a39726f40a 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -66,10 +66,15 @@
_value = Union[_scalar, _nonscalar]
TreeValue = Union[_value, 'Annotated[_value]']
-# This is an alias for an arbitrary JSON object, represented here as a Dict.
-# It is stricter on the value type than the recursive definition above.
-# It is used to represent SchemaInfo-related structures exclusively.
-_DObject = Dict[str, object]
+# These types are based on structures defined in QEMU's schema, so we lack
+# precise types for them here. Python 3.6 does not offer TypedDict constructs,
+# so they are broadly typed here as simple Python Dicts.
+SchemaInfo = Dict[str, object]
+SchemaInfoObject = Dict[str, object]
+SchemaInfoObjectVariant = Dict[str, object]
+SchemaInfoObjectMember = Dict[str, object]
+SchemaInfoCommand = Dict[str, object]
+
_NodeT = TypeVar('_NodeT', bound=_value)
@@ -162,7 +167,7 @@ def __init__(self, prefix: str, unmask: bool):
' * QAPI/QMP schema introspection', __doc__)
self._unmask = unmask
self._schema: Optional[QAPISchema] = None
- self._trees: List[Annotated[_DObject]] = []
+ self._trees: List[Annotated[SchemaInfo]] = []
self._used_types: List[QAPISchemaType] = []
self._name_map: Dict[str, str] = {}
self._genc.add(mcgen('''
@@ -234,9 +239,18 @@ def _gen_features(features: List[QAPISchemaFeature]
) -> List[Annotated[str]]:
return [Annotated(f.name, f.ifcond) for f in features]
- def _gen_tree(self, name: str, mtype: str, obj: _DObject,
+ def _gen_tree(self, name: str, mtype: str, obj: Dict[str, object],
ifcond: List[str],
features: Optional[List[QAPISchemaFeature]]) -> None:
+ """
+ Build and append a SchemaInfo object to self._trees.
+
+ :param name: The entity's name.
+ :param mtype: The entity's meta-type.
+ :param obj: Additional entity fields, as appropriate for the meta-type.
+ :param ifcond: List of conditionals that apply to this entire entity.
+ :param features: Optional features field for SchemaInfo.
+ """
comment: Optional[str] = None
if mtype not in ('command', 'event', 'builtin', 'array'):
if not self._unmask:
@@ -251,8 +265,8 @@ def _gen_tree(self, name: str, mtype: str, obj: _DObject,
self._trees.append(Annotated(obj, ifcond, comment))
def _gen_member(self, member: QAPISchemaObjectTypeMember
- ) -> Annotated[_DObject]:
- obj: _DObject = {
+ ) -> Annotated[SchemaInfoObjectMember]:
+ obj: SchemaInfoObjectMember = {
'name': member.name,
'type': self._use_type(member.type)
}
@@ -262,13 +276,9 @@ def _gen_member(self, member: QAPISchemaObjectTypeMember
obj['features'] = self._gen_features(member.features)
return Annotated(obj, member.ifcond)
- def _gen_variants(self, tag_name: str,
- variants: List[QAPISchemaVariant]) -> _DObject:
- return {'tag': tag_name,
- 'variants': [self._gen_variant(v) for v in variants]}
-
- def _gen_variant(self, variant: QAPISchemaVariant) -> Annotated[_DObject]:
- obj: _DObject = {
+ def _gen_variant(self, variant: QAPISchemaVariant
+ ) -> Annotated[SchemaInfoObjectVariant]:
+ obj: SchemaInfoObjectVariant = {
'case': variant.name,
'type': self._use_type(variant.type)
}
@@ -300,11 +310,12 @@ def visit_object_type_flat(self, name: str, info: Optional[QAPISourceInfo],
features: List[QAPISchemaFeature],
members: List[QAPISchemaObjectTypeMember],
variants: Optional[QAPISchemaVariants]) -> None:
- obj: _DObject = {'members': [self._gen_member(m) for m in members]}
+ obj: SchemaInfoObject = {
+ 'members': [self._gen_member(m) for m in members]
+ }
if variants:
- obj.update(self._gen_variants(variants.tag_member.name,
- variants.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)
def visit_alternate_type(self, name: str, info: Optional[QAPISourceInfo],
@@ -329,7 +340,7 @@ def visit_command(self, name: str, info: Optional[QAPISourceInfo],
arg_type = arg_type or self._schema.the_empty_object_type
ret_type = ret_type or self._schema.the_empty_object_type
- obj: _DObject = {
+ obj: SchemaInfoCommand = {
'arg-type': self._use_type(arg_type),
'ret-type': self._use_type(ret_type)
}
--
2.29.2
John Snow <jsnow@redhat.com> writes:
> Add some aliases that declare intent for some of the "dictly-typed"
> objects we pass around in introspect.py.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
>
> ---
>
> This patch is optional, it can be dropped if desired. If it's taken,
> it's probably best to squash it with the prior patch. It is purely for
> the sake of demonstrating what the _DObject was meant to convey: a
> Python Dict that represents some JSON Object. It does not add any type
> safety in and of itself, but does have some (light) annotational
> benefit. In this case, it's usually a specific data structure from the
> QAPI Schema we are referencing, but do not have "compile-time" access to
> in Python.
>
> These are loosely typed and don't bother reproducing the exact structure
> of the real types. Python 3.6 does not have support for TypedDict
> structures, so this is as good as we can do without involving a
> third-party library (e.g. Pydantic), in which we might be able to say:
>
>
> class SchemaMetaType(str, enum.Enum):
> BUILTIN = "builtin"
> ENUM = "enum"
> ARRAY = "array"
> OBJECT = "object"
> ALTERNATE = "alternate"
> COMMAND = "command"
> EVENT = "event"
>
>
> class SchemaInfo(pydantic.BaseModel):
> name: str
> meta-type: SchemaMetaType
> features: Optional[List[str]]
> data: Union[SchemaInfoBuiltin, SchemaInfoEnum, SchemaInfoArray,
> SchemaInfoObject, SchemaInfoAlternate, SchemaInfoCommand,
> SchemaInfoEvent]
>
>
> However, the cost of reproducing and keeping these structure definitions
> in-sync between the user-defined portion of the schema and the code
> generator is likely not worth doing any such thing. However, it does
> illustrate an interesting dependency the generator has on the
> user-defined schema itself in terms of types.
>
> So, I settled on using some light types that suggest the form of the
> object instead of enforcing the form.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> scripts/qapi/introspect.py | 51 +++++++++++++++++++++++---------------
> 1 file changed, 31 insertions(+), 20 deletions(-)
>
> diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
> index 3afcdda7446..2a39726f40a 100644
> --- a/scripts/qapi/introspect.py
> +++ b/scripts/qapi/introspect.py
> @@ -66,10 +66,15 @@
> _value = Union[_scalar, _nonscalar]
> TreeValue = Union[_value, 'Annotated[_value]']
>
> -# This is an alias for an arbitrary JSON object, represented here as a Dict.
> -# It is stricter on the value type than the recursive definition above.
> -# It is used to represent SchemaInfo-related structures exclusively.
> -_DObject = Dict[str, object]
> +# These types are based on structures defined in QEMU's schema, so we lack
> +# precise types for them here. Python 3.6 does not offer TypedDict constructs,
> +# so they are broadly typed here as simple Python Dicts.
> +SchemaInfo = Dict[str, object]
> +SchemaInfoObject = Dict[str, object]
> +SchemaInfoObjectVariant = Dict[str, object]
> +SchemaInfoObjectMember = Dict[str, object]
> +SchemaInfoCommand = Dict[str, object]
> +
>
> _NodeT = TypeVar('_NodeT', bound=_value)
>
> @@ -162,7 +167,7 @@ def __init__(self, prefix: str, unmask: bool):
> ' * QAPI/QMP schema introspection', __doc__)
> self._unmask = unmask
> self._schema: Optional[QAPISchema] = None
> - self._trees: List[Annotated[_DObject]] = []
> + self._trees: List[Annotated[SchemaInfo]] = []
> self._used_types: List[QAPISchemaType] = []
> self._name_map: Dict[str, str] = {}
> self._genc.add(mcgen('''
> @@ -234,9 +239,18 @@ def _gen_features(features: List[QAPISchemaFeature]
> ) -> List[Annotated[str]]:
> return [Annotated(f.name, f.ifcond) for f in features]
>
> - def _gen_tree(self, name: str, mtype: str, obj: _DObject,
> + def _gen_tree(self, name: str, mtype: str, obj: Dict[str, object],
> ifcond: List[str],
> features: Optional[List[QAPISchemaFeature]]) -> None:
> + """
> + Build and append a SchemaInfo object to self._trees.
> +
> + :param name: The entity's name.
> + :param mtype: The entity's meta-type.
> + :param obj: Additional entity fields, as appropriate for the meta-type.
> + :param ifcond: List of conditionals that apply to this entire entity.
> + :param features: Optional features field for SchemaInfo.
> + """
> comment: Optional[str] = None
> if mtype not in ('command', 'event', 'builtin', 'array'):
> if not self._unmask:
> @@ -251,8 +265,8 @@ def _gen_tree(self, name: str, mtype: str, obj: _DObject,
> self._trees.append(Annotated(obj, ifcond, comment))
>
> def _gen_member(self, member: QAPISchemaObjectTypeMember
> - ) -> Annotated[_DObject]:
> - obj: _DObject = {
> + ) -> Annotated[SchemaInfoObjectMember]:
> + obj: SchemaInfoObjectMember = {
> 'name': member.name,
> 'type': self._use_type(member.type)
> }
> @@ -262,13 +276,9 @@ def _gen_member(self, member: QAPISchemaObjectTypeMember
> obj['features'] = self._gen_features(member.features)
> return Annotated(obj, member.ifcond)
>
> - def _gen_variants(self, tag_name: str,
> - variants: List[QAPISchemaVariant]) -> _DObject:
> - return {'tag': tag_name,
> - 'variants': [self._gen_variant(v) for v in variants]}
> -
> - def _gen_variant(self, variant: QAPISchemaVariant) -> Annotated[_DObject]:
> - obj: _DObject = {
> + def _gen_variant(self, variant: QAPISchemaVariant
> + ) -> Annotated[SchemaInfoObjectVariant]:
> + obj: SchemaInfoObjectVariant = {
> 'case': variant.name,
> 'type': self._use_type(variant.type)
> }
> @@ -300,11 +310,12 @@ def visit_object_type_flat(self, name: str, info: Optional[QAPISourceInfo],
> features: List[QAPISchemaFeature],
> members: List[QAPISchemaObjectTypeMember],
> variants: Optional[QAPISchemaVariants]) -> None:
> - obj: _DObject = {'members': [self._gen_member(m) for m in members]}
> + obj: SchemaInfoObject = {
> + 'members': [self._gen_member(m) for m in members]
> + }
> if variants:
> - obj.update(self._gen_variants(variants.tag_member.name,
> - variants.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)
>
> def visit_alternate_type(self, name: str, info: Optional[QAPISourceInfo],
> @@ -329,7 +340,7 @@ def visit_command(self, name: str, info: Optional[QAPISourceInfo],
>
> arg_type = arg_type or self._schema.the_empty_object_type
> ret_type = ret_type or self._schema.the_empty_object_type
> - obj: _DObject = {
> + obj: SchemaInfoCommand = {
> 'arg-type': self._use_type(arg_type),
> 'ret-type': self._use_type(ret_type)
> }
I see what you mean by "some (light) annotational benefit".
I figure some of is simply due to going from _DObject, a name that tells
me nothing, to SchemaInfo, which makes me go aha!
The remainder is having the subtypes of SchemaInfo, too. albeit without
actual type checking. Worthwhile?
I agree it should be squashed if we want it, or parts of it.
On 2/8/21 10:43 AM, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:
>
>> Add some aliases that declare intent for some of the "dictly-typed"
>> objects we pass around in introspect.py.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>>
>> ---
>>
>> This patch is optional, it can be dropped if desired. If it's taken,
>> it's probably best to squash it with the prior patch. It is purely for
>> the sake of demonstrating what the _DObject was meant to convey: a
>> Python Dict that represents some JSON Object. It does not add any type
>> safety in and of itself, but does have some (light) annotational
>> benefit. In this case, it's usually a specific data structure from the
>> QAPI Schema we are referencing, but do not have "compile-time" access to
>> in Python.
>>
>> These are loosely typed and don't bother reproducing the exact structure
>> of the real types. Python 3.6 does not have support for TypedDict
>> structures, so this is as good as we can do without involving a
>> third-party library (e.g. Pydantic), in which we might be able to say:
>>
>>
>> class SchemaMetaType(str, enum.Enum):
>> BUILTIN = "builtin"
>> ENUM = "enum"
>> ARRAY = "array"
>> OBJECT = "object"
>> ALTERNATE = "alternate"
>> COMMAND = "command"
>> EVENT = "event"
>>
>>
>> class SchemaInfo(pydantic.BaseModel):
>> name: str
>> meta-type: SchemaMetaType
>> features: Optional[List[str]]
>> data: Union[SchemaInfoBuiltin, SchemaInfoEnum, SchemaInfoArray,
>> SchemaInfoObject, SchemaInfoAlternate, SchemaInfoCommand,
>> SchemaInfoEvent]
>>
>>
>> However, the cost of reproducing and keeping these structure definitions
>> in-sync between the user-defined portion of the schema and the code
>> generator is likely not worth doing any such thing. However, it does
>> illustrate an interesting dependency the generator has on the
>> user-defined schema itself in terms of types.
>>
>> So, I settled on using some light types that suggest the form of the
>> object instead of enforcing the form.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>> scripts/qapi/introspect.py | 51 +++++++++++++++++++++++---------------
>> 1 file changed, 31 insertions(+), 20 deletions(-)
>>
>> diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
>> index 3afcdda7446..2a39726f40a 100644
>> --- a/scripts/qapi/introspect.py
>> +++ b/scripts/qapi/introspect.py
>> @@ -66,10 +66,15 @@
>> _value = Union[_scalar, _nonscalar]
>> TreeValue = Union[_value, 'Annotated[_value]']
>>
>> -# This is an alias for an arbitrary JSON object, represented here as a Dict.
>> -# It is stricter on the value type than the recursive definition above.
>> -# It is used to represent SchemaInfo-related structures exclusively.
>> -_DObject = Dict[str, object]
>> +# These types are based on structures defined in QEMU's schema, so we lack
>> +# precise types for them here. Python 3.6 does not offer TypedDict constructs,
>> +# so they are broadly typed here as simple Python Dicts.
>> +SchemaInfo = Dict[str, object]
>> +SchemaInfoObject = Dict[str, object]
>> +SchemaInfoObjectVariant = Dict[str, object]
>> +SchemaInfoObjectMember = Dict[str, object]
>> +SchemaInfoCommand = Dict[str, object]
>> +
>>
>> _NodeT = TypeVar('_NodeT', bound=_value)
>>
>> @@ -162,7 +167,7 @@ def __init__(self, prefix: str, unmask: bool):
>> ' * QAPI/QMP schema introspection', __doc__)
>> self._unmask = unmask
>> self._schema: Optional[QAPISchema] = None
>> - self._trees: List[Annotated[_DObject]] = []
>> + self._trees: List[Annotated[SchemaInfo]] = []
>> self._used_types: List[QAPISchemaType] = []
>> self._name_map: Dict[str, str] = {}
>> self._genc.add(mcgen('''
>> @@ -234,9 +239,18 @@ def _gen_features(features: List[QAPISchemaFeature]
>> ) -> List[Annotated[str]]:
>> return [Annotated(f.name, f.ifcond) for f in features]
>>
>> - def _gen_tree(self, name: str, mtype: str, obj: _DObject,
>> + def _gen_tree(self, name: str, mtype: str, obj: Dict[str, object],
>> ifcond: List[str],
>> features: Optional[List[QAPISchemaFeature]]) -> None:
>> + """
>> + Build and append a SchemaInfo object to self._trees.
>> +
>> + :param name: The entity's name.
>> + :param mtype: The entity's meta-type.
>> + :param obj: Additional entity fields, as appropriate for the meta-type.
>> + :param ifcond: List of conditionals that apply to this entire entity.
>> + :param features: Optional features field for SchemaInfo.
>> + """
>> comment: Optional[str] = None
>> if mtype not in ('command', 'event', 'builtin', 'array'):
>> if not self._unmask:
>> @@ -251,8 +265,8 @@ def _gen_tree(self, name: str, mtype: str, obj: _DObject,
>> self._trees.append(Annotated(obj, ifcond, comment))
>>
>> def _gen_member(self, member: QAPISchemaObjectTypeMember
>> - ) -> Annotated[_DObject]:
>> - obj: _DObject = {
>> + ) -> Annotated[SchemaInfoObjectMember]:
>> + obj: SchemaInfoObjectMember = {
>> 'name': member.name,
>> 'type': self._use_type(member.type)
>> }
>> @@ -262,13 +276,9 @@ def _gen_member(self, member: QAPISchemaObjectTypeMember
>> obj['features'] = self._gen_features(member.features)
>> return Annotated(obj, member.ifcond)
>>
>> - def _gen_variants(self, tag_name: str,
>> - variants: List[QAPISchemaVariant]) -> _DObject:
>> - return {'tag': tag_name,
>> - 'variants': [self._gen_variant(v) for v in variants]}
>> -
>> - def _gen_variant(self, variant: QAPISchemaVariant) -> Annotated[_DObject]:
>> - obj: _DObject = {
>> + def _gen_variant(self, variant: QAPISchemaVariant
>> + ) -> Annotated[SchemaInfoObjectVariant]:
>> + obj: SchemaInfoObjectVariant = {
>> 'case': variant.name,
>> 'type': self._use_type(variant.type)
>> }
>> @@ -300,11 +310,12 @@ def visit_object_type_flat(self, name: str, info: Optional[QAPISourceInfo],
>> features: List[QAPISchemaFeature],
>> members: List[QAPISchemaObjectTypeMember],
>> variants: Optional[QAPISchemaVariants]) -> None:
>> - obj: _DObject = {'members': [self._gen_member(m) for m in members]}
>> + obj: SchemaInfoObject = {
>> + 'members': [self._gen_member(m) for m in members]
>> + }
>> if variants:
>> - obj.update(self._gen_variants(variants.tag_member.name,
>> - variants.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)
>>
>> def visit_alternate_type(self, name: str, info: Optional[QAPISourceInfo],
>> @@ -329,7 +340,7 @@ def visit_command(self, name: str, info: Optional[QAPISourceInfo],
>>
>> arg_type = arg_type or self._schema.the_empty_object_type
>> ret_type = ret_type or self._schema.the_empty_object_type
>> - obj: _DObject = {
>> + obj: SchemaInfoCommand = {
>> 'arg-type': self._use_type(arg_type),
>> 'ret-type': self._use_type(ret_type)
>> }
>
> I see what you mean by "some (light) annotational benefit".
>
> I figure some of is simply due to going from _DObject, a name that tells
> me nothing, to SchemaInfo, which makes me go aha!
>
Naming is hard! Using the *exact* names seems helpful.
> The remainder is having the subtypes of SchemaInfo, too. albeit without
> actual type checking. Worthwhile?
>
For referential purposes it seems useful, at least in so far as
declaring intent. When I was trying to type this series, I was not aware
this module was building structures defined in introspect.json until
quite late into the series.
I kept wondering: "What are the valid keys here? Who consumes this?" --
It wasn't obvious.
I'm leaning towards just keeping them all for the documentation benefit,
though I admit it *could* mislead someone into believing it's type
checked when it isn't, really. I tried to call that out with the comment
at the top, which I hope suffices to diffuse that potential
faith-based-landmine.
> I agree it should be squashed if we want it, or parts of it.
>
Squash all of it, IMO.
--js
© 2016 - 2026 Red Hat, Inc.