Add Missing Type Annotations #40
Conversation
| @@ -185,7 +193,7 @@ def set_iaq_relative_humidity(self, celsius, relative_humidity): | |||
|
|
|||
| # Low level command functions | |||
|
|
|||
| def _run_profile(self, profile): | |||
| def _run_profile(self, profile: Tuple[str, List[int], int, float]) -> List[int]: | |||
There was a problem hiding this comment.
I didn't review everything but thought it would be helpful to mention this first - technically this library uses lists for profile instead of tuple so it's not correct. However, the way that this function unpacks the argument, I think tuples are a much better fit, and we should change all uses of this function to use tuples instead of lists. Is that something you would be interesting in help with? It'll make the type annotations much smoother and sensible.
There was a problem hiding this comment.
well, based on a comment to another pr, wouldn't we fix this to be lists for annotations and do the change to tuple in another pr? I think I used tuple here because of hints pycharm was giving me and also the fact I was having problems wrapping my head around the structure of the profile being a list. which says more about me and my understanding of lists, I suppose. so yes, I should dig a bit and do that. not before next week, tho. In here or a different PR?
There was a problem hiding this comment.
I would do it here, unlike the other PR, changing those lists to tuples wouldn't be API-breaking, just minor modifications to aid in type annotations. I agree with you that that it just doesn't make sense for it to be a list, since we're unpacking it to a fixed set of variables anyway.
|
submitted for review, changing passed lists to tuples |
Updating https://github.com/adafruit/Adafruit_CircuitPython_SGP30 to 3.0.6 from 3.0.5: > Merge pull request adafruit/Adafruit_CircuitPython_SGP30#40 from tcfranks/main Updating https://github.com/adafruit/Adafruit_CircuitPython_ImageLoad to 1.16.8 from 1.16.7: > Merge pull request adafruit/Adafruit_CircuitPython_ImageLoad#64 from tekktrik/dev/use-unified-workflows Updating https://github.com/adafruit/Adafruit_CircuitPython_LED_Animation to 2.6.0 from 2.5.20: > Merge pull request adafruit/Adafruit_CircuitPython_LED_Animation#101 from arturo182/patch-1 Updating https://github.com/adafruit/Adafruit_CircuitPython_Logging to 5.0.0 from 4.2.0: > Merge pull request adafruit/Adafruit_CircuitPython_Logging#41 from isacben/fix-default-logging-level
resolves #34
submitted for review / comment