chrysn / aiocoap

The Python CoAP library
Other
262 stars 119 forks source link

Fix or suppress MyPy errors #336

Closed Codeglitches closed 1 month ago

Codeglitches commented 5 months ago

As discussed in #335 the code currently does not fair well when the types are analyzed by MyPy. This pull request makes some changes so that it does pass a MyPy analysis check without errors. This would be a good starting point to start working on adding Annotations throughout the code.

Not all errors are really solved. In a few places a dynamic code style is used or there is some type confusion. These errors that could not be solved without some more refactoring are suppressed using the # type: ignore comment.

Let me know what you think of the changes

chrysn commented 1 month ago

Thanks, that's a useful contribution; still looking through it.

Testing locally, mypy complains about cbor2 missing annotations (Skipping analyzing "cbor2": module is installed, but missing library stubs or py.typed marker). Were you using any particular branch thereof? (I tried with having the whl built from latest cbor2 master in my PYTHONPATH).

If I want to test this in CI, is there anything more than mypy aiocoap to test? (answered already in https://github.com/chrysn/aiocoap/issues/335#issuecomment-1892126945)

I've recently dropped support for Python 3.7, 3.8 and 3.9; did you apply any workarounds to support type checking in old versions, or is that something that rather happens on the boxes of developers anyway, where a recent Python is present (and at runtime on older Pythons, type information is discarded)?

chrysn commented 1 month ago

Apart from the experiments in #347, I've pushed some more changes:

chrysn commented 1 month ago

To summarize the current state, there are two things I'd like to do differently still:

Sorry for the delay in taking this up; are you still around and would you address those?

chrysn commented 1 month ago

The enum rework of #347 appears to work, allowing the enum definitions to themselves to stay easy -- one down.

chrysn commented 1 month ago

The one large copied list is now also verified, and I'm in the process of looking through the remaining ignores.

chrysn commented 1 month ago

... and the large swaths are fixed by overhauling typing in oscore (where this exercise actually helped find a bug).

Running a few last checks, but then I think this can go in together with #347.

Codeglitches commented 1 month ago

Thanks for merging this pull request. Sorry for not lending a hand on finishing it up properly; I was otherwise occupied.