On Fri, Feb 10, 2023, 11:26 AM John Snow <js...@redhat.com> wrote:
>
>
> On Fri, Feb 10, 2023, 10:44 AM Markus Armbruster <arm...@redhat.com>
> wrote:
>
>> John Snow <js...@redhat.com> writes:
>>
>> > mypy can only narrow the type of `Mapping[str, ...].keys() & Set[str]`
>> > to `AbstractSet[str]` and not a `Set[str]`. As a result, if the type of
>> > an expression is changed to a Mapping[], mypy is unsure if the .pop() is
>> > safe.
>> >
>> > A forthcoming commit does exactly that, so wrap the expression in a
>> > set() constructor to force the intermediate expression to be resolved as
>> > a mutable type.
>> >
>> > Signed-off-by: John Snow <js...@redhat.com>
>> > ---
>> > scripts/qapi/expr.py | 4 ++--
>> > 1 file changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
>> > index b56353bdf84..af802367eff 100644
>> > --- a/scripts/qapi/expr.py
>> > +++ b/scripts/qapi/expr.py
>> > @@ -622,8 +622,8 @@ def check_expr(expr_elem: _JSONObject) -> None:
>> > if 'include' in expr:
>> > return
>> >
>> > - metas = expr.keys() & {'enum', 'struct', 'union', 'alternate',
>> > - 'command', 'event'}
>> > + metas = set(expr.keys() & {
>> > + 'enum', 'struct', 'union', 'alternate', 'command', 'event'})
>> > if len(metas) != 1:
>> > raise QAPISemError(
>> > info,
>> "expression must have exactly one key"
>> " 'enum', 'struct', 'union', 'alternate',"
>> " 'command', 'event'")
>> meta = metas.pop()
>>
>> I'm mildly surprised that the result of operator & is considered
>> immutable. How could it *not* be a freshly created set? Oh well.
>>
>
> I think because it's up to the LHS type to return the result.
>
> Wait, maybe I can just switch the operand order, lol.
>
Good news, I can just switch the operand order. Looks cleaner.
>
>> Passing a set to set() is a no-op, so
>>
>> Reviewed-by: Markus Armbruster <arm...@redhat.com>
>>
>> Regardless, the code feels a bit too clever to me. It actually did
>> already when I wrote it, but I the less clever ways I could think of
>> were also much more verbose.
>>
>> The code checks that exactly one of a set of keys is present, and which
>> one it is. Any ideas?
>>
>
> Yeah, it feels too clever but I'm not sure I know something more elegant,
> really. I'll consider it while I address your feedback and prepare a respin.
>
Still don't have brighter ideas. A problem for another time.
