From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 7C977C636D4 for ; Wed, 15 Feb 2023 14:47:29 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1pSJ3S-0001JX-Mv; Wed, 15 Feb 2023 09:46:42 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pSJ3Q-0001J1-Cg for qemu-devel@nongnu.org; Wed, 15 Feb 2023 09:46:40 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pSJ3O-0000zV-Eb for qemu-devel@nongnu.org; Wed, 15 Feb 2023 09:46:40 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1676472397; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=i1jRXK7pkxrzD1FKYwrn6enR86u0aIAO40KinKfw0B0=; b=AORx9yQBOuwuWPr/kl6AZ6lDf6L3yAq23jmJuuVIxTuVzK0piDsaYFjtKSpsOj1+vsxpX4 qIdI1zkQCxvVdjBWIKP7ECUtBZ/yfj/EVGzYRk4Htez4ad56hTUbwEgDETfnwJ9zJniQEJ jW8fE109lOBMaYMoDL6TwO9fNjiOAOU= Received: from mail-pg1-f200.google.com (mail-pg1-f200.google.com [209.85.215.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-38-IT2-ObM1NkaQmvQHN9W4NQ-1; Wed, 15 Feb 2023 09:46:35 -0500 X-MC-Unique: IT2-ObM1NkaQmvQHN9W4NQ-1 Received: by mail-pg1-f200.google.com with SMTP id x3-20020a654143000000b004f2f2dafb21so7206989pgp.0 for ; Wed, 15 Feb 2023 06:46:35 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=i1jRXK7pkxrzD1FKYwrn6enR86u0aIAO40KinKfw0B0=; b=Zkwr5Qge0W2kvBcT2Ua1iMz628eDxPzYQvoWEROr1lvpWUr3af/GqlMaK0Wmdnadrb 23KNXWcPNcQtdRvC+GuZYPTZ4Wc+zT2rNvpqB37wAzbvmj5UWg+pYHvnr6e/VOGLTe0o EmHBFDr+rauTCT3wE0167qvzpo/4JlFugJD/5KG1G/xQquRnFaoibhzqGiEX7MLqyBEi Z1fRSpgesFCTYwT1kLr7BerGPF/8OM8RtLwOzhv9GcPlqtKLP/IbByPGJjFwinIHo3ht eLdxG2SXDaSGTReEMWREZMCLmn714pPKDRrpciifhNXkQbkOOszL2f+Anh2oCdg4uI2j mmLg== X-Gm-Message-State: AO0yUKU0keOmD7xCx3BvrHc7pEBM6AY6hJCyYUx0IWMQMoWXSnV6No2A /jp14cs497LLDuLicPesUZvUs5bwhUbvHzIbZmTGhBF9P4/W2vjTpfDoBUK+JvlqP+h4k9GPan5 3ZwLFX8I9F7NhhSt0MJw5kLjroFfo3Bs= X-Received: by 2002:a63:b141:0:b0:4f1:ccba:5bac with SMTP id g1-20020a63b141000000b004f1ccba5bacmr375448pgp.20.1676472394355; Wed, 15 Feb 2023 06:46:34 -0800 (PST) X-Google-Smtp-Source: AK7set8uSYaPgNc/EOVQdHKw/9P4zQUxeTCua+ZqRJy2Mgfs6aKBIqeywiNmIKXU0W65lFeeBwyy//5ViXXuP11gp2U= X-Received: by 2002:a63:b141:0:b0:4f1:ccba:5bac with SMTP id g1-20020a63b141000000b004f1ccba5bacmr375440pgp.20.1676472393982; Wed, 15 Feb 2023 06:46:33 -0800 (PST) MIME-Version: 1.0 References: <20230215000011.1725012-1-jsnow@redhat.com> <20230215000011.1725012-4-jsnow@redhat.com> <87a61fe6x6.fsf@pond.sub.org> <87cz6b82z7.fsf@pond.sub.org> In-Reply-To: <87cz6b82z7.fsf@pond.sub.org> From: John Snow Date: Wed, 15 Feb 2023 09:46:23 -0500 Message-ID: Subject: Re: [PATCH v4 3/6] qapi: Add minor typing workaround for 3.6 To: Markus Armbruster Cc: qemu-devel , Michael Roth Content-Type: multipart/alternative; boundary="00000000000084fe8105f4be2544" Received-SPF: pass client-ip=170.10.133.124; envelope-from=jsnow@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org --00000000000084fe8105f4be2544 Content-Type: text/plain; charset="UTF-8" On Wed, Feb 15, 2023, 8:36 AM Markus Armbruster wrote: > Markus Armbruster writes: > > > John Snow writes: > > > >> Pylint under 3.6 does not believe that Collection is subscriptable at > >> runtime. It is, making this a Pylint > >> bug. https://github.com/PyCQA/pylint/issues/2377 > >> > >> They closed it as fixed, but that doesn't seem to be true as of Pylint > >> 2.13.9, the latest version you can install under Python 3.6. 2.13.9 was > >> released 2022-05-13, about seven months after the bug was closed. > >> > >> The least-annoying fix here is to just use the more specific type > >> Sequence, only because it seems to work in 3.6. > >> > >> Signed-off-by: John Snow > >> --- > >> scripts/qapi/expr.py | 6 +++--- > >> 1 file changed, 3 insertions(+), 3 deletions(-) > >> > >> diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py > >> index 5a1782b57ea..8701351fdfc 100644 > >> --- a/scripts/qapi/expr.py > >> +++ b/scripts/qapi/expr.py > >> @@ -33,11 +33,11 @@ > >> > >> import re > >> from typing import ( > >> - Collection, > >> Dict, > >> Iterable, > >> List, > >> Optional, > >> + Sequence, > >> Union, > >> cast, > >> ) > >> @@ -195,8 +195,8 @@ def check_defn_name_str(name: str, info: > QAPISourceInfo, meta: str) -> None: > >> def check_keys(value: _JSONObject, > >> info: QAPISourceInfo, > >> source: str, > >> - required: Collection[str], > >> - optional: Collection[str]) -> None: > >> + required: Sequence[str], > >> + optional: Sequence[str]) -> None: > >> """ > >> Ensure that a dict has a specific set of keys. > > > > The actual arguments are always List[str]. You actually used that until > > v3 of the patch, and switched to the maximally general Collection[str] > > in v4, with rationale that ended up in commit 538cd41065a: > > > > qapi/expr.py: Modify check_keys to accept any Collection > > > > This is a minor adjustment that lets parameters @required and > > @optional take tuple arguments, in particular (). Later patches will > > make use of that. > > > > No later patch ever did. > I have some in "pt5d" that do - but this is the set of patches that do some optional cleanups that you've dropped from earlier sets. > > > I'd prefer maximally stupid List[str], but it's no big deal either way. > > Regardless, > Reviewed-by: Markus Armbruster > Thanks- everything else look OK enough for now? > --00000000000084fe8105f4be2544 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


On Wed, Feb 15, 2023, 8:36 AM Markus Armbruster <armbru@redhat.com> wrote:
Markus Armbruster <armbru@redhat.com= > writes:

> John Snow <jsnow@redhat.com> writes:
>
>> Pylint under 3.6 does not believe that Collection is subscriptable= at
>> runtime. It is, making this a Pylint
>> bug. https://github.com/PyCQA/pylin= t/issues/2377
>>
>> They closed it as fixed, but that doesn't seem to be true as o= f Pylint
>> 2.13.9, the latest version you can install under Python 3.6. 2.13.= 9 was
>> released 2022-05-13, about seven months after the bug was closed.<= br> >>
>> The least-annoying fix here is to just use the more specific type<= br> >> Sequence, only because it seems to work in 3.6.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>=C2=A0 scripts/qapi/expr.py | 6 +++---
>>=C2=A0 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
>> index 5a1782b57ea..8701351fdfc 100644
>> --- a/scripts/qapi/expr.py
>> +++ b/scripts/qapi/expr.py
>> @@ -33,11 +33,11 @@
>>=C2=A0
>>=C2=A0 import re
>>=C2=A0 from typing import (
>> -=C2=A0 =C2=A0 Collection,
>>=C2=A0 =C2=A0 =C2=A0 Dict,
>>=C2=A0 =C2=A0 =C2=A0 Iterable,
>>=C2=A0 =C2=A0 =C2=A0 List,
>>=C2=A0 =C2=A0 =C2=A0 Optional,
>> +=C2=A0 =C2=A0 Sequence,
>>=C2=A0 =C2=A0 =C2=A0 Union,
>>=C2=A0 =C2=A0 =C2=A0 cast,
>>=C2=A0 )
>> @@ -195,8 +195,8 @@ def check_defn_name_str(name: str, info: QAPIS= ourceInfo, meta: str) -> None:
>>=C2=A0 def check_keys(value: _JSONObject,
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0info:= QAPISourceInfo,
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0sourc= e: str,
>> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0required: = Collection[str],
>> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0optional: = Collection[str]) -> None:
>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0required: = Sequence[str],
>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0optional: = Sequence[str]) -> None:
>>=C2=A0 =C2=A0 =C2=A0 """
>>=C2=A0 =C2=A0 =C2=A0 Ensure that a dict has a specific set of keys.=
>
> The actual arguments are always List[str].=C2=A0 You actually used tha= t until
> v3 of the patch, and switched to the maximally general Collection[str]=
> in v4, with rationale that ended up in commit 538cd41065a:
>
>=C2=A0 =C2=A0 =C2=A0qapi/expr.py: Modify check_keys to accept any Colle= ction
>=C2=A0 =C2=A0 =C2=A0
>=C2=A0 =C2=A0 =C2=A0This is a minor adjustment that lets parameters @re= quired and
>=C2=A0 =C2=A0 =C2=A0@optional take tuple arguments, in particular ().= =C2=A0 Later patches will
>=C2=A0 =C2=A0 =C2=A0make use of that.
>
> No later patch ever did.

I have some in "pt5d" that do - but = this is the set of patches that do some optional cleanups that you've d= ropped from earlier sets.

>
> I'd prefer maximally stupid List[str], but it's no big deal ei= ther way.

Regardless,
Reviewed-by: Markus Armbruster <armbru@redhat.com>

Thanks- everythi= ng else look OK enough for now?
--00000000000084fe8105f4be2544--