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.

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

typing overhaul + pre-commit updates #200

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

elpekenin
Copy link

STILL WIP!!!

For a bit of a background, Im planning to go off the list of open typing issues and fix stuff. First one in the list was some BLE-related one, and when i cloned it and started working, noticed that mypy wasn't ignoring this (adafruit_ble) module due to the lack of py.typed on it. Then decided to open a quick PR to simply add such empty marker file... But, surprise, typing was very broken, so here we are, fixing it ;)

Notes

  • Perhaps should undo changes to optional-requirements.txt
  • Depends on changes i've PRed to the circuitpython repository itself (a couple of wrong type-hints in doc comments in the C code), i locally patched my circuitpython-stubs to run the tests
  • Added assert on some places to concrete Union and Optional down to one of their options.
    • There are now a few lines of code running assert foo is not None before doing foo.bar on the very next line.
    • I'm retty sure these asserts will not break code that was working until now.
    • Letting you know just in case
  • Testing is welcome, i dont have any BLE boards
  • py.typed will be added on the final commit, when every single file passes the checks

While working on this, i've made a couple upgrades to pre-commit's config

  • Add isort
  • Upgrade pylint (old version does not build/install on Py312)
  • Add mypy

Right now, only a couple errors are left. These, for some reason, are not spotted when running pre-commit run --all-files but i get them running $ mypy adafruit_ble manually. Would like to know what the cause for this is, maybe related to pre-commit using its own virtual environment....

@elpekenin elpekenin marked this pull request as ready for review August 22, 2024 22:13
@elpekenin elpekenin marked this pull request as draft August 24, 2024 01:26
@elpekenin
Copy link
Author

Working on better mypy integration

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant