Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix or suppress MyPy errors #336

Merged
merged 29 commits into from
May 13, 2024
Merged

Fix or suppress MyPy errors #336

merged 29 commits into from
May 13, 2024

Conversation

Codeglitches
Copy link
Contributor

@Codeglitches Codeglitches commented Jan 21, 2024 •
edited
Loading

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

@Codeglitches
Fix MyPy Error due to dynamic code generation
e9cd163
@Codeglitches
Fix MyPy error as the urlparse library no longer exists in python3
8bc3d9e
@Codeglitches
Add type annotation as required by MyPy
090bad1
@Codeglitches
Suppress MyPy error as the module can not be imported
674334e
@Codeglitches
Ignore incompatible types in assignment
2c8cf81
@Codeglitches
Fix: missing attributes
dc78ca5
@Codeglitches
Add mypy override for modules that connot be imported or are missing …
2531358 
…type information
@Codeglitches
Add required annotation for _items
b2ec040
@Codeglitches
Fix incompatible type in assignment
7090ef9
@Codeglitches
Fix Argument 1 of _requires_ua has incompatible type
3523db6
@Codeglitches
Max definitions equal to those in RFC8323Remote as they are both inhe…
8c8bb73 
…rited by WSRemote
@Codeglitches
suppress annotation-unchecked notes
2d5b4b1
@Codeglitches
suppress attr-defined for supported_ct
84bc8d6
@Codeglitches
Annotate OptionNumbers enum values
be7dd87 
All annotations need to be suppressed however, as they are seen as int
@Codeglitches
Fix: MyPy errors due to MyPy not seeing the code module under numbers
7d3fa84
@Codeglitches
Add required annotation for variable
fdd8924
@Codeglitches
Add missing attributes
97fae40
@Codeglitches
Fix assigning different types to same variable
0c1f9ad
@Codeglitches
Ignore import errors
bc2c296
@Codeglitches
ignore type mismatch
1a36736
@Codeglitches
Fix: MyPy errors due to MyPy not seeing the code module under numbers
9de84eb
@Codeglitches
Fix wrong type annotation
b7ac505
@Codeglitches
suppress attr-defined for from_item
be3757e
@Codeglitches
Fix MyPy signature incompatible with supertype warning
206d9b7
@Codeglitches
Ignore MyPy warning due to use of assignment with property
d5a0cb1
@Codeglitches
Fix direct imports from dynamic generated __all__
a7fe67a
@chrysn chrysn mentioned this pull request May 4, 2024
Closed
@chrysn
Copy link
Owner

chrysn commented May 6, 2024 •
edited
Loading

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 #335 (comment))

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
remotes: Allow writing maximum_block_size_exp for some types
5c3ac19 
This reverts a regression on setting per-remote-instance
maximum_block_size_exp introduced in 8c8bb73.
@chrysn chrysn mentioned this pull request May 6, 2024
Merged
@chrysn
Copy link
Owner

chrysn commented May 6, 2024

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

  • AIU Any is frowned upon; I think I found the right types to go there.
  • I've merged in master, which also gives it a shot at CI

@chrysn
Copy link
Owner

chrysn commented May 7, 2024

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

  • Wherever lists of things are spelled out rather than done programmatically, there should be an if __debug__: guarded check that verifies them, and possibly offers code generation
  • There are some large swaths of type: ignore that I'll go through, maybe I can provide better hints
  • The enum situation could need enhancement, optionnumbers are becoming unsightly

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

@chrysn
Copy link
Owner

chrysn commented May 11, 2024

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

@chrysn
Copy link
Owner

chrysn commented May 11, 2024

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

@chrysn
Copy link
Owner

chrysn commented May 12, 2024

... 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.

chrysn added a commit that referenced this pull request May 13, 2024
@chrysn
Add support for mypy
8950da3 
This includes some non-trivial changes in how attributes are exposed and
how enums are generated, none of which should have publicly observable
effects unless the behavior was unreliable already.

Notable changes include:

* ExtensibleIntEnum types (Code, ContentFormat) are now based on
  Python's `enum.Enum`.
* Group OSCORE: Fixed crash when attempting to use cipher suite that
  does not support all modes.
* OSCORE contexts' `external_aad_is_group` property was replaced with
  subclassing `ContextWhereExternalAadIsGroup`

Merges: #336
Merges: #347
@chrysn chrysn merged commit 5c9fded into chrysn:master May 13, 2024
@chrysn chrysn mentioned this pull request May 13, 2024
Closed
@Codeglitches
Copy link
Contributor Author

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

@Codeglitches Codeglitches deleted the support-mypy branch May 24, 2024 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chrysn chrysn chrysn left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Codeglitches @chrysn