Re: [PATCH 16/37] qapi: establish mypy type-checking baseline

[John Snow]

On 9/21/20 4:05 AM, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:
>
> > On 9/18/20 7:55 AM, Markus Armbruster wrote:
> > > Ignorant question: why does this come after PATCH 13 "qapi/common.py:
> > > add notational type hints", but before all the other patches adding type
> > > hints?
> > > John Snow <jsnow@redhat.com> writes:
> > >
> > > > Fix two very minor issues, and then establish a mypy type-checking
> > > > baseline.
> > > >
> > > > Like pylint, this should be run from the folder above:
> > > >
> > > >    > mypy --config-file=qapi/mypy.ini qapi/
> > > I get:
> > >       $ mypy --config-file qapi/mypy.ini qapi
> > >       qapi/mypy.ini: [mypy]: Strict mode is not supported in configuration 
> > > files: specify individual flags instead (see 'mypy -h' for the list of flags 
> > > enabled in strict mode)
> > >       qapi/types.py:29: error: Need type annotation for 'objects_seen' (hint: 
> > > "objects_seen: Set[<type>] = ...")
> > >       Found 1 error in 1 file (checked 18 source files)
> > > Is this expected?
> > > In case it matters:
> > >       $ mypy --version
> > >       mypy 0.761
> >
> > (Warning; FiSH and stgit ahead)
> >
> > cd ~/src/qemu/scripts
> > pipenv --python 3.6
> > pipenv shell
> > pip install pylint flake8
> >
> > ### Testing mypy 0.770
> >
> > pip install mypy==0.770
> > stg goto qapi-establish-mypy-type
> >
> > while true; and flake8 qapi/; and pylint --rcfile=qapi/pylintrc qapi/;
> > and mypy --config-file=qapi/mypy.ini qapi/; and stg push; end
> >
> > pip uninstall mypy
> >
> > ###
> >
> >
> >
> > 0.782 - OK
> > 0.770 - OK
> > 0.760 - FAIL, Fixable*
> > 0.750 - OK*
> > 0.740 - OK*
> > 0.730 - OK*
> > 0.720 - OK*
> > 0.710 - OK** (Does not recognize 'no_implicit_reexport' option)
> > 0.700 - OK*** (Not compatible with bleeding edge pylint/flake8)
> > 0.670 - OK***
> > 0.660 - OK***
> > 0.650 - OK***
> > 0.641 - OK***
> > 0.630 - Fails again.
> >
> >
> >
> > 0.760 doesn't support strict in the config file (It needs component
> > options), and it wants a few extra annotations where its inference
> > power is weaker. Well, easy enough to fix up.
> >
> > 0.630 fails again and insists that __init__ should have a return type
> > annotation of None. Modern mypy is smart enough to know that's what
> > that type is supposed to be. Arbitrarily, this is my cutoff for what
> > seems reasonable to even want to support.
> >
> > I still find the lack of "strict=true" in the config file irritating
> > and might ask to target 0.770 or newer. There should be no reason we
> > can't install that in a venv for CI to chew on.
> >
> > Humbly ask I take the lazy way out and document that we support mypy
> > > = 0.770.
>
> Our "supported build platforms" policy puts hard limits on the minimum
> versions for tools the build requires.
>
> Mypy is not such a tool.  I hope we get to the point where we can have
> "make check" run it, but skipping the test when we don't have a
> sufficiently modern mypy feels okay to me, as long as our gating CI
> still guards the master branch.

Yes, that's been my view. These tools *are* new and they *do* change 
often. Chasing a wide compatibility window with them is prohibitively 
difficult. Luckily, they're not needed for running the packages at all.

If I do add linting to 'make check', it is going to be through a virtual 
environment that deploys *specific versions* of these tools, and it will 
happen transparently. The VM tests already do this.

--js