-
Notifications
You must be signed in to change notification settings - Fork 6
Add Type Annotations #15
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.
I have one requested change to the way the import is written. The rest of this is looking good to me.
Thanks for working on this @sokratisvas!
adafruit_vcnl4040.py
Outdated
try: | ||
from busio import I2C | ||
except ImportError: | ||
pass |
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.
For the import here we can try importing typing first like this:
try:
import typing
from busio import I2C
except ImportError:
pass
the typing
module doesn't exist on microcontrollers so this will fail to import and then prevent it from importing busio
(or anything else that we would need strictly for typing rather than functionality). Which will save a bit of RAM space for us
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.
Thanks for the detailed answer! When I added import typing
in the try except block, the pre-commit test fails at pylint (library code)
. More specifically, the pylint error is adafruit_vcnl4040.py:42:4: W0611: Unused import typing (unused-import)
. Should I commit the changes anyway?
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.
Yup, typically pylint
would be correct, but the behavior @FoamyGuy detailed is more important. You can disable that specific warning using a comment:
try:
import typing # pylint: disable=unused-import
from busio import I2C
except ImportError:
pass
That'll let pylint
know you understand it's anger but don't care 😆
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.
Nice! Thanks @sokratisvas and @FoamyGuy!
Updating https://github.com/adafruit/Adafruit_CircuitPython_VCNL4040 to 1.2.12 from 1.2.11: > Merge pull request adafruit/Adafruit_CircuitPython_VCNL4040#15 from sokratisvas/add-type-annotations > Fix version strings in workflow files > Update version string Updating https://github.com/adafruit/Adafruit_CircuitPython_HID to 5.3.0 from 5.2.5: > Merge pull request adafruit/Adafruit_CircuitPython_HID#100 from Neradoc/get-last-received-report > Fix version strings in workflow files > Update version string Updating https://github.com/adafruit/Adafruit_CircuitPython_MiniMQTT to 5.4.0 from 5.3.3: > Merge pull request adafruit/Adafruit_CircuitPython_MiniMQTT#116 from vladak/socket_timeout_tunable > Fix version strings in workflow files > Update version string
Hi, here is my attempt to fix #14 .