-
Notifications
You must be signed in to change notification settings - Fork 74
__init__ arguments are not Pins #121
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
Comments
@dhalbert I think dropping the suffix would be clearer compared to substituting |
Is this something we want to do? I'm happy to submit a PR for this :) Was also planning to add type annotations, if that helps at all. |
resolved by #146 |
@tekktrik all of the other instances of similar These ones do have Learn Guide code using the named keyword arguments as well, so we will need to make sure to update all usages at the same time as the library changes are made. |
I agree with @brentru that dropping the suffix is clearer than changing it to |
I can submit another PR to drop it if that's the case |
I think it's important to point out that by taking DigitalInOut its usually meant to work with GPIO expander provided objects as well. Pin should be only used when a native pin object is expected. |
The arg names here are suffixed as
_pin
, but they should actually beDigitalInOut
objects. The arg names could be clearer, maybe_dio
or just drop the suffix. At least one user might have been confused by this: https://forums.adafruit.com/viewtopic.php?f=60&t=173353.Adafruit_CircuitPython_ESP32SPI/adafruit_esp32spi/adafruit_esp32spi.py
Lines 136 to 138 in fce466b
The text was updated successfully, but these errors were encountered: