kmk_firmware
kmk_firmware copied to clipboard
Consider static type checking
Probably something like https://github.com/Microsoft/pyright (or maybe https://pyre-check.org) would help us sure up a lot of the insanity that happens at our interface boundaries.
I don't think Micro/Circuit Python support PEP-526, so we'd be leaning on PEP-484 and whatever subset of pyright's features can fit PEP-484 syntax, but that's alright. Alternatively, we could check to see if there's a Python 3.10 to 3.5 transpiler out there that would strip, say, type aliases or rewrite pattern matches, at compile time (given that my day job is TypeScript, I'm innately familiar with this concept, but I don't know if it's made its way into Python-land yet. It could be an interesting project to write if it doesn't exist, though...)
Another thing worth noting is that we'd need to ensure that https://github.com/python/typeshed (or a similar collection we and/or Adafruit maintains - this might be something to chat with @tannewt about) has type stubs for CircuitPython's stdlib, which would be a fairly non-trivial undertaking. We only use a small subset of CircuitPython's API, though, so doing a partial type stub writeup probably wouldn't be nightmarish.
Other rationale: we basically don't have automated tests (very early on I started down the road of trying to do functional testing, but mocking out the MicroPython APIs proved tricky, so eventually they were removed from the tree), so a type system would at least ensure all the proverbial lego bricks were the right shape, even if not validating their behaviors precisely.
The typeshed equivalent for CircuitPython is the circuitpython-stubs
package: https://pypi.org/project/circuitpython-stubs/#history
We just started releasing it automatically.
@klardotsh @kdb424 as we've discussed over in Matrix I'm going to try to take this on.
I do have a few questions that I feel are worth keeping in here just so we can track them.
- Is there a target Python version I should be working for?
- As stated above by @klardotsh
"I don't think Micro/Circuit Python support PEP-526, so we'd be leaning on PEP-484 and whatever subset of pyright's features can fit PEP-484 syntax"
I'll work on getting the codebase typed and then use thecircuitpython-stubs
package when needed. - I believe
hid.py
was a file that maybe I should avoid- are there any other places worth avoiding? - Any really great places to start?
I'll stay in touch via Matrix but if anything important comes up pertaining to this thread I'll be sure to move conversation here.
Thanks!
Circuitpython mostly targets 3.4, but has some extras like fstrings from 3.6 thanks to klardotsh. HID is somewhere we should avoid as it will eventually need replaced by upstreams hid anyways, but otherwise, sounds reasonable to me.
Probably the best places to start are the most recent files: anything pertaining to modules (kmk/modules/__init__.py
and all sibling files) should be fairly straightforward to type out, as should kmk/kmktime.py
, kmk/types.py
and kmk/consts.py
. From there..... you'd have to dive into the beast that is kmk/kmk_keyboard.py
, which will probably cause the most mayhem, as there's still some tangly bits in there, and because it calls into all those other files (so you'll get validation over whether the type signatures are actually representative of current behavior)
See #226 for my current draft PR.
I opened an issue (#497) before seeing this one. I was under the impression CircuitPython didn't support PEP-526's type annotations, but I accidentally instinctively included some type hinting in some of my code and didn't run into any issues. They're using standard type hinting on this Adafruit page about typing in CircuitPython. They also import typing
to use its helper classes during development, but ignore the ImportError
at runtime.
Am I missing something? If not, I'd like to go ahead and start adding type hinting to KMK as mentioned in the issue I opened, using the work @sofubi started as reference.
I'm fine with bumping CircuitPy minimum version to whatever added support for that PEP if we can figure out when it happened (or just say "current latest is new minimum", if that's not too much community turbulence). This would be dope - I've largely moved off of Python in everything in my life in favor of stronger-typed languages and have been pushing type systems' use at work (Ruby) heavily; I definitely don't need sold on the benefits to approve someone adopting that branch :)
It looks like support for PEP-526 type annotation was merged in this PR, according to this issue, which was included starting from version 7.0. 7.0's full release occurred less than one month after the previous work on adding type annotations - that's unfortunate. I would not object to continually targeting the latest stable version, but I like living on the edge.
Regardless, I will begin work on adding type hints.
We're using typing in our CircuitPython libraries now so it should be fine here too.
Thanks @tannewt, helpful context that KMK won't be the odd one out for taking PEP-526 sigs into mainline!
And thanks @Jawfish for diving back into this! I'll be excited to get some review eyes on this whenever you have something to share.
Tangentially, on KMK side: Much like the Rust community tracks Minimum Supported Rust Version (MSRV), we should probably track this in the main readme of KMK - a few times now we've done major bumps to take on new CPy features (the last big one was F-strings way back when, so it's at least infrequent), and we should be front-and-center about those deps. Right now that info is buried in the Getting Started sub-doc, and is pretty loosey-goosey in its wording ("It is best to use the last stable version (>7.0).")
I've found myself being unable to give this the time and effort that I would like to lately. I intend to chip away at it as I find time to, but I figured I would mention here that it's slow going in case any other interested parties come across this conversation and would be otherwise dissuaded.