adobe-type-tools
Add typing to otfautohint
Description
otfautohint is a complex piece of code, and it's easy to get lost; it's even easier to make mistakes. Having typing information available makes it easier to catch such mistakes, and allows editors with Python type inferencing (Visual Code with pylance, Sublime Text with pyright, etc.) to provide type hints for variables and methods. This in turn helps to find typos, dead code, and so on.
Checklist:
- [X] I have followed the Contribution Guidelines
- [ ] ~I have added test code and data to prove that my code functions correctly~ (N/A)
- [X] I have verified that new and existing tests pass locally with my changes
- [X] I have performed a self-review of my own code
- [X] I have commented my code, particularly in hard-to-understand areas
- [ ] ~I have made corresponding changes to the documentation~ (N/A)
As you're probably realizing this has a lot of formatting related problems (flake8), some of which are due to edits for line length that you've removed.
@simoncozens thanks for this contribution. It looks like there are a number of linter failures that are blocking the tests from running. I recommend that you run flake8 using the config from this repo, fix all reported issues, and resubmit. Once we get the linter + tests green we will have a closer review and see about getting it merged.
Yeah, sorry, I was assuming the code formatting was black/PEP8 so my editor runs black on save to keep things tidy. I'll try and put things back into your preferred style.
I think I would prefer a submission with the substantive changes and then (perhaps) a different submission with the large amount of stylistic changes. Or perhaps just skip the latter? Best to either mesh one's contributions with the existing style or talk in advance about what you want to change and why.
Edit: Ninja-ed.
@josh-hadley Merging this would presumably require additional pytype github checks, so that notations like # pytype: disable=attribute-error won't just rot.
To be honest, it doesn't currently work with pytype. I started working with pytype, and as I added annotations I found myself in situations where a method returned an instance of its own class, which needs the Self type, and pytype doesn't currently support that. So I switched to mypy. In a couple of cases mypy doesn't like the Self return value, but it does a better job than pytype (and continues running, whereas pytype just stops and refuses to go on).
Coverage tests are sad because apparently you can't use weakref.ReferenceType as a generic in Python 3.8.
One reason we ported psautohint to python was the hope that it could be modified by the broader community, in comparison to the C code which was only ever worked on by a small group of Adobe-internal folks while squinting hard and crossing their fingers.
This PR is both an external contribution and an effort to make further contributions easier, so in that sense it's what we want and were hoping for.
Still, there is a basic concern about keeping these annotations from rotting. Some users will have environments that will make good use of the typing. Others will just be editing the files and running the tests and may have no idea what this stuff is or, if they do, how to change the annotations as the code changes.
Obviously some sort of verifier that can be run as part of the GitHub checks would be ideal, but it sounds like pytype can't do that and mypy might or might not be able to.
Do you have thoughts on this? Basically, how to maintain types in Python assuming a wide variety of users with different levels of experience and tooling?
Also: Keep in mind that we haven't been doing this internally, so it make take a while just to digest the implications of typed Python on our end.
FWIW annotations are pretty useless unless they are actually checked by a typechecker
ok, well.. self-documentation sure, but they then tend to rot as @skef noted.
Obviously some sort of verifier that can be run as part of the GitHub checks would be ideal, but it sounds like pytype can't do that and mypy might or might not be able to.
We can certainly get mypy set up in the CI, and add a disable comment to turn off the Self checks.