[PATCH v3 0/8] qapi: static typing conversion, pt4

John Snow posted 8 patches 3 years ago
Failed in applying to current master (apply log)
docs/sphinx/qapidoc.py |  3 ++-
scripts/qapi/error.py  | 47 ++++++++++++++++++++++++------------------
scripts/qapi/mypy.ini  |  5 -----
scripts/qapi/parser.py | 14 ++++++++++++-
scripts/qapi/pylintrc  |  3 +--
scripts/qapi/schema.py |  4 ++--
6 files changed, 45 insertions(+), 31 deletions(-)
[PATCH v3 0/8] qapi: static typing conversion, pt4
Posted by John Snow 3 years ago
Hi, this series adds static type hints to the QAPI module.
This is part four, and focuses on error.py.

Part 4: https://gitlab.com/jsnow/qemu/-/tree/python-qapi-cleanup-pt4
CI: https://gitlab.com/jsnow/qemu/-/pipelines/290152364

Requirements:
- Python 3.6+
- mypy >= 0.770
- pylint >= 2.6.0 (2.7.0+ when using Python 3.9+)

Every commit should pass with:
 - `isort -c qapi/`
 - `flake8 qapi/`
 - `pylint --rcfile=qapi/pylintrc qapi/`
 - `mypy --config-file=qapi/mypy.ini qapi/`

V3:

```
001/8:[down]      'qapi/error: Repurpose QAPIError as an abstract base exception class'
002/8:[----] [--] 'qapi/error: Use Python3-style super()'
003/8:[----] [--] 'qapi/error: Make QAPISourceError 'col' parameter optional'
004/8:[down]      'qapi/error: assert QAPISourceInfo is not None'
005/8:[0010] [FC] 'qapi/error.py: move QAPIParseError to parser.py'
006/8:[----] [--] 'qapi/error.py: enable pylint checks'
007/8:[----] [-C] 'qapi/error: Add type hints'
008/8:[----] [--] 'qapi/error.py: enable mypy checks'
```

- 01: Updated commit message. (Is it sufficient to pass the sniff test?)
- 04: Changed commit title/message.
- 05: Added a small docstring (I know, hubris again) to add a small
      breadcrumb to the new error location for QAPIParseError.

John Snow (8):
  qapi/error: Repurpose QAPIError as an abstract base exception class
  qapi/error: Use Python3-style super()
  qapi/error: Make QAPISourceError 'col' parameter optional
  qapi/error: assert QAPISourceInfo is not None
  qapi/error.py: move QAPIParseError to parser.py
  qapi/error.py: enable pylint checks
  qapi/error: Add type hints
  qapi/error.py: enable mypy checks

 docs/sphinx/qapidoc.py |  3 ++-
 scripts/qapi/error.py  | 47 ++++++++++++++++++++++++------------------
 scripts/qapi/mypy.ini  |  5 -----
 scripts/qapi/parser.py | 14 ++++++++++++-
 scripts/qapi/pylintrc  |  3 +--
 scripts/qapi/schema.py |  4 ++--
 6 files changed, 45 insertions(+), 31 deletions(-)

-- 
2.30.2



Re: [PATCH v3 0/8] qapi: static typing conversion, pt4
Posted by Markus Armbruster 3 years ago
John Snow <jsnow@redhat.com> writes:

> Hi, this series adds static type hints to the QAPI module.
> This is part four, and focuses on error.py.
>
> Part 4: https://gitlab.com/jsnow/qemu/-/tree/python-qapi-cleanup-pt4
> CI: https://gitlab.com/jsnow/qemu/-/pipelines/290152364
>
> Requirements:
> - Python 3.6+
> - mypy >= 0.770
> - pylint >= 2.6.0 (2.7.0+ when using Python 3.9+)
>
> Every commit should pass with:
>  - `isort -c qapi/`
>  - `flake8 qapi/`
>  - `pylint --rcfile=qapi/pylintrc qapi/`
>  - `mypy --config-file=qapi/mypy.ini qapi/`

I doubt PATCH 1 is worth the trouble without an actual concrete
location-less error.  However, arguing about it some more would also not
worth the trouble, so:

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

and queued.  Thanks!


Re: [PATCH v3 0/8] qapi: static typing conversion, pt4
Posted by John Snow 3 years ago
On 4/22/21 4:09 AM, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:
> 
>> Hi, this series adds static type hints to the QAPI module.
>> This is part four, and focuses on error.py.
>>
>> Part 4: https://gitlab.com/jsnow/qemu/-/tree/python-qapi-cleanup-pt4
>> CI: https://gitlab.com/jsnow/qemu/-/pipelines/290152364
>>
>> Requirements:
>> - Python 3.6+
>> - mypy >= 0.770
>> - pylint >= 2.6.0 (2.7.0+ when using Python 3.9+)
>>
>> Every commit should pass with:
>>   - `isort -c qapi/`
>>   - `flake8 qapi/`
>>   - `pylint --rcfile=qapi/pylintrc qapi/`
>>   - `mypy --config-file=qapi/mypy.ini qapi/`
> 
> I doubt PATCH 1 is worth the trouble without an actual concrete
> location-less error.  However, arguing about it some more would also not
> worth the trouble, so:
> 
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> 
> and queued.  Thanks!
> 

FWIW, I use it a few times in Part 5a:

- Once in main.py, to wrap an OSError (You'll see why)
- Again in qapidoc to create a "QAPIDocError" class, but this is only 
used in PoC code to demonstrate that QAPISchemaParser is fully typed.

I am finding it handy, though it may be true we wind up not needing it, 
based on reviews in part 5.

Thanks for queueing it up!

--js

(Why does tbird spellcheck not think 'queueing' is a word?)