Re: [PATCH v4 11/14] qapi/introspect.py: add type hint annotations

[John Snow <jsnow@redhat.com>]

On 2/3/21 10:15 AM, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:
> 
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>   scripts/qapi/introspect.py | 115 ++++++++++++++++++++++++++-----------
>>   scripts/qapi/mypy.ini      |   5 --
>>   scripts/qapi/schema.py     |   2 +-
>>   3 files changed, 82 insertions(+), 40 deletions(-)
>>
>> diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
>> index 60ec326d2c7..b7f2a6cf260 100644
>> --- a/scripts/qapi/introspect.py
>> +++ b/scripts/qapi/introspect.py
>> @@ -30,10 +30,19 @@
>>   )
>>   from .gen import QAPISchemaMonolithicCVisitor
>>   from .schema import (
>> +    QAPISchema,
>>       QAPISchemaArrayType,
>>       QAPISchemaBuiltinType,
>> +    QAPISchemaEntity,
>> +    QAPISchemaEnumMember,
>> +    QAPISchemaFeature,
>> +    QAPISchemaObjectType,
>> +    QAPISchemaObjectTypeMember,
>>       QAPISchemaType,
>> +    QAPISchemaVariant,
>> +    QAPISchemaVariants,
>>   )
>> +from .source import QAPISourceInfo
>>   
>>   
>>   # This module constructs a tree data structure that is used to
>> @@ -57,6 +66,8 @@
>     # generate the introspection information for QEMU. It behaves similarly
>     # to a JSON value.
>     #
>     # A complexity over JSON is that our values may or may not be annotated.
>     #
>     # Un-annotated values may be:
>     #     Scalar: str, bool, None.
>     #     Non-scalar: List, Dict
>     # _value = Union[str, bool, None, Dict[str, TreeValue], List[TreeValue]]
>     #
>     # With optional annotations, the type of all values is:
>     # TreeValue = Union[_value, Annotated[_value]]
>     #
>     # Sadly, mypy does not support recursive types, so we must approximate this.
>     _stub = Any
>     _scalar = Union[str, bool, None]
>     _nonscalar = Union[Dict[str, _stub], List[_stub]]
>>   _value = Union[_scalar, _nonscalar]
>>   TreeValue = Union[_value, 'Annotated[_value]']
>>   
>> +# This is a (strict) alias for an arbitrary object non-scalar, as above:
>> +_DObject = Dict[str, object]
> 
> Sounds greek :)
> 

Admittedly it is still not explained well ... until the next patch. I'm 
going to leave it alone for now until you have a chance to respond to 
these walls of text.

> It's almost the Dict part of _nonscalar, but not quite: object vs. Any.
> 
> I naively expect something closer to
> 
>     _scalar = ...
>     _object = Dict[str, _stub]
>     _nonscalar = Union[_object, List[_stub]
> 
> and (still naively) expect _object to be good enough to serve as type
> annotation for dicts representing JSON objects.

"_object" would be good, except ... I am trying to avoid using that word 
because what does it mean? Python object? JSON object? Here at the 
boundary between two worlds, nothing makes sense.

(See patch 12/14 for A More Betterer Understanding of what _DObject is 
used for. It will contribute to A Greater Understanding.)

Anyway, to your questions;

(1) _DObject was my shorthand garbage way of saying "This is a Python 
Dict that represents a JSON object". Hence Dict-Object, "DObject". I 
have also derisively called this a "dictly-typed" object at times.

(2) Dict[str, Any] and Dict[str, object] are similar, but do have a 
semantic difference. I alluded to it by calling this a "(strict) alias"; 
which does not help you understand any of the following points:

Whenever you use "Any", it basically turns off type-checking at that 
boundary; it is the gradually typed boundary type. Avoid it whenever 
reasonably possible.

Example time:


def foo(thing: Any) -> None:
     print(thing.value)  # Sure, I guess! We'll believe you.


def foo(thing: object) -> None:
     print(thing.value)  # BZZT, Python object has no value prop.


Use "Any" when you really just cannot constrain the type, because you're 
out of bourbon or you've decided to give in to the darkness inside your 
heart.

Use "object" when the type of the value actually doesn't matter, because 
you are only passing it on to something else later that will do its own 
type analysis or introspection on the object.

For introspect.py, 'object' is actually a really good type when we can 
use it, because we interrogate the type exhaustively upon receipt in 
_tree_to_qlit.


That leaves one question you would almost assuredly ask as a followup:

"Why didn't you use object for the stub type to begin with?"

Let's say we define _stub as `object` instead, the Python object. When 
_tree_to_qlit recurses on non-scalar structures, the held value there is 
only known as "object" and not as str/bool/None, which causes a typing 
error at that point.

Moving the stub to "Any" tells mypy to ... not worry about what type we 
actually passed here. I gave in to the darkness in my heart. It's just 
too annoying without real recursion.

--js