Am 13.03.19 um 00:34 schrieb Gregory Farnum: > We talked about this briefly in the testing weekly last week and it > looks pretty cool to me! As a C++ developer dabbling in Python, I've > had periodic trouble dealing with dynamic typing (o_0) and the extra > tests we could add against PRs seem real nice. > > Unfortunately mypy is apparently pretty noisy and has a number of > false positives if you *don't* add the annotations, so we can't just > use it for free. It's actually worse: mypy needs type annotations to properly type check Python code. * If we're not annotating `mgr/ceph_module.BaseMgrModule`, it's unable to find non-existing methods in all mgr modules. That's the reason I added a `ceph_module.pyi` in https://github.com/ceph/cep/pull/26769 In general it's unable to type check code which originates from C++ or cython. * mypy is unable to deduce function signatures: > $ cat f.py > def f(x): > return x + 1 > > f("hello") > $ mypy f.py > $ python3 f.py > Traceback (most recent call last): > File "f.py", line 4, in <module> > f("hello") > File "f.py", line 2, in f > return x + 1 > TypeError: must be str, not int Compared with a version with type annotations: > $ cat f.py > def f(x: int) -> int: > return x + 1 > > f("hello") > $ mypy f.py > f.py:4: error: Argument 1 to "f" has incompatible type "str"; expected "int" Which means we have to annotate a _lot_ of functions. Especially the ones that are called multiple times. > > How do people feel about adding those annotations to our coding > standards and enforcing rules on PRs about not adding warnings? > Perhaps we could try it out in just the mgr, or one of the modules, if > we feel like we need some experience with the tradeoffs? Personally, I wouldn't want to force anyone to add function signatures or type annotations. But, as a side note, we can also make use of type annotations for documentation purposes, if we enable the sphinx-autodoc-typehints extension in sphinx. That means we already have four real and existing use cases for type annotations: 1. Type signatures are also a form of regular code documentation 2. We can use it in sphinx to enhance our public API documentation 3. And every bit of type annotation increases mypy's coverage. 4. Type signatures are parsed by some IDEs (e.g. PyCharm) to highlight type errors > -Greg > > On Mon, Mar 4, 2019 at 1:17 AM Sebastian Wagner <swagner@xxxxxxxx> wrote: >> >> all, >> >> to find bugs in our Python code base a bit earlier, I looked into static >> type checking using mypy and I'd like to share my results. >> >> Mypy ( http://mypy-lang.org/ ) is a static type checker for Python that >> aims to combine the benefits of dynamic (or "duck") typing and static >> typing. >> >> As of now, running mypy on our code base already reveals some genuine >> bugs, especially with respect to Python 3 incompatibilities. >> >> Unfortunately, we will need to look into every module and message and >> manually separate bugs and false-positives. >> >> Here are two example errors: >> >>> pybind/mgr/restful/decorators.py:21: error: >>> Argument 1 to "split" of "bytes" has incompatible type "str"; expected "Optional[bytes]" >> >> >> This is an issue that will be fixed by >> https://github.com/ceph/ceph/pull/26712 >> >>> pybind/mgr/diskprediction_cloud/module.py:12: error: >>> Module 'string' has no attribute 'maketrans' >> >> >> which is already tracked as https://tracker.ceph.com/issues/38521 >> >> By adding additional type annotations to our Python code, we could >> enhance the quality of static type checking and thus reveal additional >> bugs. This is of course not mandatory! >> >> >> Here is the PR that adds a script that calls mypy: >> https://github.com/ceph/ceph/pull/26715 >> >> It also contains a list of messages printed by mypy. > -- SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
