chrysn / aiocoap

The Python CoAP library
Other
267 stars 120 forks source link

Adding Type Annotations and Package Type Information #335

Closed Codeglitches closed 6 months ago

Codeglitches commented 10 months ago

Aiocoap currently does not contain type annotations, nor does it provide Package Type Information.

By adding type annotations to variables, functions and methods:

When oaicoap is used in a project that does use type annotations and a static type checker, its benefits for that project are greatly reduced, as oaicaop does not provide the type information and all oaicoap types are treated as "Any". By adding the type information to oaicoap those projects would benefit too.

Although it is possible to add the type information externally in typeshed I would recommend annotating the code itself as it will be easier to maintain and increases the readability as mentioned above.

By the way, type annotations have greatly improved in each new version of python since there introduction in python 3.5 so dropping older python version and bump support to 3.9 and newer as suggested in #275 sounds like a good suggesting.

chrysn commented 10 months ago

aiocoap is already using those in some places newer places (eg. aiocoap/util/cryptography_additions.py), but those are few so far.

I'm open to adding them, but I'm unfamiliar with the tooling around them. For use throughout the code base, I'd need a tool that I can wrap around tests, which will perform type checking at run time and report stack traces of occurrences when annotations are inaccurate. Can you recommend such a tool? It'd need to run on the different Python versions I'm running CI on (can probably ditch the 3.7 soon, but pypy is important) to ensure that code paths that are version dependent are also considered.

Without such checks, I think that the downsides of wrong annotations might outweigh the upsides.

Even then, additon would not be immediate (though I'm open to PRs for that provided tooling is right), but I'd then add it as code is constantly being updated.

Codeglitches commented 10 months ago

I'm open to adding them, but I'm unfamiliar with the tooling around them. For use throughout the code base, I'd need a tool that I can wrap around tests, which will perform type checking at run time and report stack traces of occurrences when annotations are inaccurate. Can you recommend such a tool?

I would recommend using mypy. It does however do static code analysis so no need to wrap it around tests. If you get to the point where all the library code is annotated and you can run mypy with --strict you get near perfect typechecking.

From the mypy documentation:

If you run mypy with the --strict flag, you will basically never get a type related error at runtime without a corresponding mypy error, unless you explicitly circumvent mypy somehow

It is easy to include in your CI/CD pipeline. To check for different python versions, either run it using a python interpreter of the version you want to check or pass the --python-version flag. If required, you also have the option so select the platform in a similar way.

Mypy allows for gradually adding type annotations as by default it does not check dynamic code (code without annotations). You can specifiy per module if you will allow dynamic functions, so you could add full annotations per module and then get mypy to make sure no dynamic functions are added in the future. See Using MyPy with an existing codebase for more information on the subject.

chrysn commented 10 months ago

Hm, this is spewing a lot of errors that are effects of using static analysis on a dynamic language -- eg.

aiocoap/oscore_sitewrapper.py:17: error: Module "aiocoap.numbers.codes" has no attribute "FETCH"  [attr-defined]

when really aiocoap.numbers.codes sets its local FETCH dynamically after processing an enum. (In its extreme form, those attributes may come from scraped IANA registries, which would make a code generation mess). In other cases, it fails to notice that a module does guarantee some submodules' inclusion (so numbers.code.FOO is legal after from aiocoap import numbers), or not seeing through standard Python constructs like enums (OptionNumber.PROXY_URI.format = optiontypes.StringOption is legal, but mypy erroneously thinks PROXY_URI is an integer, just because it was assigned one in in the type, before the metaclass does things).

Granted, it finds actual errors too (like mismatching ABC signatures), but so far this has a terrible SNR. Are there any other tools worth recommending, or are there any (non-obvious -- I skimmed --help) options that make mypy more relaxed when it sees that objects are tampered with at runtime, and makes it fall back to generously assuming that interactions are *Any, **Any -> Any?

Codeglitches commented 10 months ago

I ran mypy against the code as well and noticed the codes errors as well and noticed the errors due to dynamic code generation too.

My IDE (PyCharm) has the same issue as it does static analysis too. Personally I don't use that type of construction; it is nice to write less code, but I loose more than I gain as it trips all static code analysis tools. As those codes will not change (maybe some will be added) just writing it out would be my approach (and when generating code the extra code would be a complete non issue, as the generator will generate it). But of course this is just my opinion on style and/or priority.

In other cases, it fails to notice that a module does guarantee some submodules' inclusion (so numbers.code.FOO is legal after from aiocoap import numbers),

I don't know why that is, but it is not mypy that complains. PyLint and PyCharm have the same complaint. But is easily fixed by importing "numbers.code" and writing "code.NOT_FOUND" instead of numbers and writing "numbers.code.NOT_FOUND" right? Possibly it can be fixed as mypy has many configuration options, but I don't know how.

So, yes some changes are required to reap all benefits of the annotations, but most of them will increase readability and/or usage in IDE's, which makes the library easier to use and to work on.

Are there any other tools worth recommending, or are there any (non-obvious -- I skimmed --help) options that make mypy more relaxed when it sees that objects are tampered with at runtime, and makes it fall back to generously assuming that interactions are *Any, **Any -> Any?

Yes you can. One of the ways is excluding the file like in mypy aiocoap --exclude=aiocoap.numbers.codes.py. You don't have to specify this on the command line every time, you can put in a mypy config section in .mypy.ini or pyproject.toml for instance. Possibly, it can be don in a different way too, but I don't know how.

chrysn commented 6 months ago

Closed through #336 and #347.