Re: [PATCH v4 4/4] qapi: expose all schema features to code

[Markus Armbruster <armbru@redhat.com>]

John Snow <jsnow@redhat.com> writes:

> On Fri, Feb 7, 2025, 6:57 AM Markus Armbruster <armbru@redhat.com> wrote:
>
>> Daniel P. Berrangé <berrange@redhat.com> writes:
>>
>> > This replaces use of the constants from the QapiSpecialFeatures
>> > enum, with constants from the auto-generate QapiFeatures enum
>> > in qapi-features.h
>> >
>> > The 'deprecated' and 'unstable' features still have a little bit of
>> > special handling, being force defined to be the 1st + 2nd features
>> > in the enum, regardless of whether they're used in the schema. This
>> > retains compatibility with common code that references the features
>> > via the QapiSpecialFeatures constants.
>> >
>> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>>
>> Daniel, feel free to ignore this at least for now.  I'm trying to learn
>> some typing lore from John.
>>
>> v3 made mypy unhappy.  I asked John for advice, and also posted a
>> solution involving ValuesView I hacked up myself.  Daniel took it for
>> v4.
>>
>> John suggested to use List.
>>
>> I now wonder whether could use Iterable.
>>
>> I'll show the three solutions inline.
>>
>> John, thoughts?
>>
>
> ValuesView works just fine. It accurately describes what that function
> returns. I only avoided it in my fixup because it's a more obscure type and
> generally list is easier to work with as a first-class built in primitive
> type to the language.
>
> (read as: I didn't have to consult any docs to fix it up using List and I'm
> lazy.)

Aside: John later shared a useful technique on IRC: "you can write
reveal_type(foo) to get mypy to spill the beans on what it thinks".

> Your solution describes precisely the type being returned (always good) and
> avoids any re-copying of data.
>
> Do be aware by caching the values view object in another object that you
> are keeping a "live reference" to the list of dict values that I think can
> change if the source dict changes.

Yes.

>                                    I doubt it matters, but you should know
> about that.

I believe it's just fine.

> The only design consideration you have now is what type you actually want
> to return and why. I think it barely matters, and I'm always going to opt
> for whatever is the least annoying for the patch author so I don't have to
> bore/torture them with python minutiae.

Since the typing in Daniel's patch is fine, I'll refrain from messing
with it.

> As long as the tests pass (my first three patches in the dan-fixup branch I
> posted based on v3) I'm more than content.

Thanks!