adafruit / Adafruit_CircuitPython_NeoPixel

CircuitPython drivers for neopixels.
MIT License
302 stars 98 forks source link

Missing Type Annotations #118

Closed FoamyGuy closed 2 years ago

FoamyGuy commented 2 years ago

There are missing type annotations for some functions in this library.

The typing module does not exist on CircuitPython devices so the import needs to be wrapped in try/except to catch the error for missing import. There is an example of how that is done here:

try:
    from typing import List, Tuple
except ImportError:
    pass

Once imported the typing annotations for the argument type(s), and return type(s) can be added to the function signature. Here is an example of a function that has had this done already:

def wrap_text_to_pixels(
    string: str, max_width: int, font=None, indent0: str = "", indent1: str = ""
) -> List[str]:

If you are new to Git or Github we have a guide about contributing to our projects here: https://learn.adafruit.com/contribute-to-circuitpython-with-git-and-github

There is also a guide that covers our CI utilities and how to run them locally to ensure they will pass in Github Actions here: https://learn.adafruit.com/creating-and-sharing-a-circuitpython-library/check-your-code In particular the pages: Sharing docs on ReadTheDocs and Check your code with pre-commit contain the tools to install and commands to run locally to run the checks.

If you are attempting to resolve this issue and need help, you can post a comment on this issue and tag both @foamyguy and @kattni or reach out to us on Discord: https://adafru.it/discord in the #circuitpython-dev channel.

The following locations are reported by mypy to be missing type annotations:

jsymons commented 2 years ago

https://github.com/adafruit/Adafruit_CircuitPython_NeoPixel/blob/202f929c51fad5135bb7bf632ef9f61bf072c0de/neopixel.py#L105

@FoamyGuy @kattni I'm looking into resolving this but it isn't exactly clear what pin is supposed to be here, there appear to be many implementations of a Pin class across the examples/dependencies. The only specific requirement seems to be that pin needs to have an id attribute for when it's passed into the DigitalInOut class. Do you just want this handled with a Protocol type class that requires an id attribute?

FoamyGuy commented 2 years ago

Thanks for working on this @jsymons I think we want microcontroller.Pin as type for the pin. Most commonly user will be passing a property from board like board.D1 or similar.

FoamyGuy commented 2 years ago

resolved by #119