Add type annotations #285
Assignees
Labels
priority:medium
Medium priority
technical debt
A feature/requirement implemented in a sub-optimal way & must be re-written. Contrast to "cleanup"
Comments
jsstevenson
added
priority:medium
|
@jsstevenson I tried to add type annotations to the files I changed in my 2-alpha update work (looking at branch |
jsstevenson
added a commit
that referenced
this issue
May 10, 2024
close #285 * Clean up types and type annotations where possible. Not exhaustive -- there's some stuff that probably needs refactoring to make progress on -- but made some good progress, such that I think we can say the "types" issue is closed enough. * **maybe important**: by virtue of where json model dumps were happening, this introduced a small change to the order of the keys in some dicts that were getting sha512'd, which meant the ID changed during testing. I set JSON serialization to first sort keys in order so that this doesn't matter in the future, but this does entail a one-time change in digest ID. if that's bad, I can revert and try to figure out a workaround. * Also changed how stuff from GKS/VRS models are imported so that the code is a little more succinct. I think `models.Variation` style imports were a relic from a Python-JSONschema-objects custom and we don't need to do it anymore.
jsstevenson
added a commit
that referenced
this issue
May 14, 2024
close #285 - Clean up types and type annotations where possible. Not exhaustive -- there's some stuff that probably needs refactoring to make progress on -- but made some good progress, such that I think we can say the "types" issue is closed enough. - maybe important: by virtue of where json model dumps were happening, this introduced a small change to the order of the keys in some dicts that were getting sha512'd, which meant the ID changed during testing. I set JSON serialization to first sort keys in order so that this doesn't matter in the future, but this does entail a one-time change in digest ID. if that's bad, I can revert and try to figure out a workaround. - Also changed how stuff from GKS/VRS models are imported so that the code is a little more succinct. I think models.Variation style imports were a relic from a Python-JSONschema-objects custom and we don't need to do it anymore.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
@korikuzma, didn't see an existing issue for this, apologies if I missed it
A lot of this code is from pretty far back in the day, and the type checker is flagging a lot of inconsistencies/missing annotations. It'd be good to get this fixed where possible.
In #284 I just totally removed checking annotation rules because it should probably be done separately. Once finished, that checker should go back on.
The text was updated successfully, but these errors were encountered: