| Message ID | 20230215000011.1725012-4-jsnow@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qapi: static typing conversion, pt5c | expand |
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/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 <jsnow@redhat.com> > --- > 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'd prefer maximally stupid List[str], but it's no big deal either way.
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/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 <jsnow@redhat.com> >> --- >> 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'd prefer maximally stupid List[str], but it's no big deal either way. Regardless, Reviewed-by: Markus Armbruster <armbru@redhat.com>
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/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 <jsnow@redhat.com> > >> --- > >> 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 <armbru@redhat.com> > Thanks- everything else look OK enough for now? >
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.
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 <jsnow@redhat.com> --- scripts/qapi/expr.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)