-
Notifications
You must be signed in to change notification settings - Fork 51
add some type annotations to adafruit_minimqtt.py #158
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for working on this @vladak! as well as all of the other improvements you've been making to this library.
I had a look over this and found a few things that I think we will want to change.
I'm also going to try it out on a device, I'll report back here with the results. Ordinarily types shouldn't change functionality at all, but there are a few cases I want to double check.
For now, I am done with the changes, unless more issues are found. Will save the more complicated stuff (Callables in particular) till next time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
Thank you @vladak
Updating https://github.com/adafruit/Adafruit_CircuitPython_floppy to 1.0.7 from 1.0.6: > Merge pull request adafruit/Adafruit_CircuitPython_floppy#9 from adafruit/pinouts Updating https://github.com/adafruit/Adafruit_CircuitPython_MiniMQTT to 7.3.2 from 7.3.1: > Merge pull request adafruit/Adafruit_CircuitPython_MiniMQTT#158 from vladak/type_annotations_mqtt Updating https://github.com/adafruit/Adafruit_CircuitPython_Bundle/circuitpython_library_list.md to NA from NA: > Updated download stats for the libraries
This change adds more type annotations.
mypy --strict adafruit_minimqtt/adafruit_minimqtt.py
currently below 100 errors (down from 168).Approaches #92.