-
Notifications
You must be signed in to change notification settings - Fork 58
Adding typing #86
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
Adding typing #86
Conversation
I'm not sure how to test the changes. Any advice? |
Looks like black made some changes. Is there a way to rerun the pipeline? |
Nice! I'll try to take a look at this soon, but maybe someone else will first! |
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.
These changes all look reasonable to me and I don’t see any problems. But I can’t find this board in my collection, so I personally would feel better if someone verified it on the hardware before merging.
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.
Feedback below! Also, would you mind adding type annotations for the properties getter and setters as well? Thanks!
I have a bbb and ads1115, but not an ads1015 |
@scirelli, can you use the generics from the |
@scirelli let me know when I should re-review |
I have not had a chance to test on actual hardware. Other than that I addressed all PR comments. Which tests should I run? I was looking at the simple one in examples. |
I also realize that I haven't reviewed the property getters and setters, I'll do that in a few hours. Sorry for the delay! |
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.
Great work!
Thanks! I learned a few things, still new to Python typing. |
Updating https://github.com/adafruit/Adafruit_CircuitPython_ADS1x15 to 2.2.21 from 2.2.19: > Merge pull request adafruit/Adafruit_CircuitPython_ADS1x15#86 from scirelli/enhancment/add_typing Updating https://github.com/adafruit/Adafruit_CircuitPython_Trellis to 1.3.15 from 1.3.14: > Merge pull request adafruit/Adafruit_CircuitPython_Trellis#20 from tcfranks/main
Adding
.venv
to the ignore list so it doesn't accidentally get addedAdding py.typed file to signify this library has typing information
Fixed typing errors found by mypy
Addresses issue #83