Add typing to LEDs #30
Conversation
There was a problem hiding this comment.
Thanks for working on this @SebastienFauque.
I had a look over it and have a few changes to recommend.
| @@ -369,7 +386,9 @@ def mix(color1, color2, weight2=0.5): | |||
| GFACTOR = 2.7 # Default gamma-correction factor for function below | |||
|
|
|||
|
|
|||
| def gamma_adjust(val, gamma_value=None, brightness=1.0, inplace=False): | |||
| def gamma_adjust( | |||
| val: Any, gamma_value: Any = None, brightness: Any = 1.0, inplace=False | |||
There was a problem hiding this comment.
brightness can be typed float and inplace can be typed with bool
val and gamma_value might be able to have something more specific with Union and a few different types, but I'm not certain. I'd need to read over the docstring some more to grok the different modes of operation.
There was a problem hiding this comment.
I hadn't seen that you asked about this function in the other comment until after I posted these.
I'll try to take a closer look tomorrow and offer what guidance I can.
I think It's also okay if we're not 100% perfect on this round of typing, things can always be refined and improved as time goes on.
There was a problem hiding this comment.
@FoamyGuy I've made the mentioned changes (made brightness obey the cases in the docstring) excluding the situation that will arise with the three unique cases on the gamma_adjust function. Case one and two are easy enough to solve, adding case 3 to the mix makes type hints difficult to build in and probably difficult for a newer user to understand.
If you have any more input on this I can go back and try to add the typing, or I can just keep it with Any.
There was a problem hiding this comment.
This one looks good to me now. Thanks again @SebastienFauque!
I think the way you've typed gamma_adjust is fine for now. The multiple modes of that function definitely make it tricky to have types for it. Perhaps someday it can be refactored and simplified.
Updating https://github.com/adafruit/Adafruit_CircuitPython_NeoPixel_SPI to 1.0.6 from 1.0.5: > Merge pull request adafruit/Adafruit_CircuitPython_NeoPixel_SPI#36 from RossK1/adding_type_hints > Add upload url to release action > Add .venv to .gitignore > Update .pylintrc for v2.15.5 > Fix release CI files > Update pylint to 2.15.5 > Updated pylint version to 2.13.0 > Switching to composite actions Updating https://github.com/adafruit/Adafruit_CircuitPython_PCF8563 to 2.0.0 from 1.0.8: > Merge pull request adafruit/Adafruit_CircuitPython_PCF8563#6 from bablokb/4upstream > Add upload url to release action > Add .venv to .gitignore > Update .pylintrc for v2.15.5 > Fix release CI files > Update pylint to 2.15.5 > Updated pylint version to 2.13.0 > Switching to composite actions Updating https://github.com/adafruit/Adafruit_CircuitPython_RGB_Display to 3.11.2 from 3.11.1: > Merge pull request adafruit/Adafruit_CircuitPython_RGB_Display#113 from tekktrik/fix/typing Updating https://github.com/adafruit/Adafruit_CircuitPython_FancyLED to 1.4.15 from 1.4.14: > Merge pull request adafruit/Adafruit_CircuitPython_FancyLED#30 from SebastienFauque/feature/typingLEDs > Add upload url to release action > Add .venv to .gitignore Updating https://github.com/adafruit/Adafruit_CircuitPython_Bundle/circuitpython_library_list.md to NA from NA: > Updated download stats for the libraries
Adds typing to fancy Leds for most functions. This solves most cases for #24.