On 10/26/20 3:42 PM, John Snow wrote:
> Hi, this series adds static type hints to the QAPI module.
> This is part two, and covers introspect.py.
>
> Part 2: https://gitlab.com/jsnow/qemu/-/tree/python-qapi-cleanup-pt2
> Everything: https://gitlab.com/jsnow/qemu/-/tree/python-qapi-cleanup-pt6
>
> - Requires Python 3.6+
> - Requires mypy 0.770 or newer (for type analysis only)
> - Requires pylint 2.6.0 or newer (for lint checking only)
>
> Type hints are added in patches that add *only* type hints and change no
> other behavior. Any necessary changes to behavior to accommodate typing
> are split out into their own tiny patches.
>
> Every commit should pass with:
> - flake8 qapi/
> - pylint --rcfile=qapi/pylintrc qapi/
> - mypy --config-file=qapi/mypy.ini qapi/
>
> V2:
> - Dropped all R-B from previous series; enough has changed.
> - pt2 is now introspect.py, expr.py is pushed to pt3.
> - Reworked again to have less confusing (?) type names
> - Added an assertion to prevent future accidental breakage
>
Ping!
Patches 1-3: Can be skipped; just enables sphinx to check the docstring
syntax. Don't worry about these too much, they're just here for you to
test with.
They are interesting, but the rebase version fails. And the error produced is not exactly friendly:
Exception occurred:
File "/usr/lib/python3.9/site-packages/sphinx/domains/c.py", line 3751, in resolve_any_xref
return [('c:' + self.role_for_objtype(objtype), retnode)]
TypeError: can only concatenate str (not "NoneType") to str
Could you rebase and split off in a separate series?
Patch 4 adds some small changes, to support:
Patch 5 adds the type hints.
Patches 6-11 try to improve the readability of the types and the code.
This was a challenging file to clean up, so I am sure there's lots of
easy, low-hanging fruit in the review/feedback for me to improve.
Nothing obvious to me.
Python typing is fairly new to me, and I don't know the best practices. I would just take what you did and improve later, if needed.
A wish item before we proceed with more python code cleanups is documentation and/or automated tests.
Could you add a new make check-python and perhaps document what the new code-style expectations?
> John Snow (11):
> [DO-NOT-MERGE] docs: replace single backtick (`) with double-backtick
> (``)
> [DO-NOT-MERGE] docs/sphinx: change default role to "any"
> [DO-NOT-MERGE] docs: enable sphinx-autodoc for scripts/qapi
> qapi/introspect.py: add assertions and casts
> qapi/introspect.py: add preliminary type hint annotations
> qapi/introspect.py: add _gen_features helper
> qapi/introspect.py: Unify return type of _make_tree()
> qapi/introspect.py: replace 'extra' dict with 'comment' argument
> qapi/introspect.py: create a typed 'Annotated' data strutcure
> qapi/introspect.py: improve readability of _tree_to_qlit
> qapi/introspect.py: Add docstring to _tree_to_qlit
>
> docs/conf.py | 6 +-
> docs/devel/build-system.rst | 120 +++++------
> docs/devel/index.rst | 1 +
> docs/devel/migration.rst | 59 +++---
> docs/devel/python/index.rst | 7 +
> docs/devel/python/qapi.commands.rst | 7 +
> docs/devel/python/qapi.common.rst | 7 +
> docs/devel/python/qapi.error.rst | 7 +
> docs/devel/python/qapi.events.rst | 7 +
> docs/devel/python/qapi.expr.rst | 7 +
> docs/devel/python/qapi.gen.rst | 7 +
> docs/devel/python/qapi.introspect.rst | 7 +
> docs/devel/python/qapi.main.rst | 7 +
> docs/devel/python/qapi.parser.rst | 8 +
> docs/devel/python/qapi.rst | 26 +++
> docs/devel/python/qapi.schema.rst | 7 +
> docs/devel/python/qapi.source.rst | 7 +
> docs/devel/python/qapi.types.rst | 7 +
> docs/devel/python/qapi.visit.rst | 7 +
> docs/devel/tcg-plugins.rst | 14 +-
> docs/devel/testing.rst | 2 +-
> docs/interop/live-block-operations.rst | 4 +-
> docs/system/arm/cpu-features.rst | 110 +++++-----
> docs/system/arm/nuvoton.rst | 2 +-
> docs/system/s390x/protvirt.rst | 10 +-
> qapi/block-core.json | 4 +-
> scripts/qapi/introspect.py | 277 +++++++++++++++++--------
> scripts/qapi/mypy.ini | 5 -
> scripts/qapi/schema.py | 2 +-
> 29 files changed, 487 insertions(+), 254 deletions(-)
> create mode 100644 docs/devel/python/index.rst
> create mode 100644 docs/devel/python/qapi.commands.rst
> create mode 100644 docs/devel/python/qapi.common.rst
> create mode 100644 docs/devel/python/qapi.error.rst
> create mode 100644 docs/devel/python/qapi.events.rst
> create mode 100644 docs/devel/python/qapi.expr.rst
> create mode 100644 docs/devel/python/qapi.gen.rst
> create mode 100644 docs/devel/python/qapi.introspect.rst
> create mode 100644 docs/devel/python/qapi.main.rst
> create mode 100644 docs/devel/python/qapi.parser.rst
> create mode 100644 docs/devel/python/qapi.rst
> create mode 100644 docs/devel/python/qapi.schema.rst
> create mode 100644 docs/devel/python/qapi.source.rst
> create mode 100644 docs/devel/python/qapi.types.rst
> create mode 100644 docs/devel/python/qapi.visit.rst
>