Add mypy#132
Conversation
ChihChengLiang
left a comment
ChihChengLiang
left a comment
There was a problem hiding this comment.
LGTM. Epic PR Han, that's a lot of improvements in typing and readabilities.
han0110
force-pushed
the
feature/mypy
branch
3 times, most recently
from
2a7f845 to
7c5bec8
Compare
|
I plan on reviewing this once #110 is merged :) |
ed255
left a comment
ed255
left a comment
There was a problem hiding this comment.
LGTM!
This PR is a great improvement in readability and usability! 🎊
Thanks @han0110 and @ChihChengLiang for working on this!
| self.message = f"Lookup {table_name} is ambiguous on inputs {inputs}, ${len(matched_rows)} matched rows found: {matched_rows}" | ||
|
|
||
|
|
||
| class TableRow: |
There was a problem hiding this comment.
These new types to express table rows are very welcome! They improve readability a lot :)
| (10, RW.Read, RWTableTag.Stack, 1, 1023, 0, b, 0, 0, 0), | ||
| (11, RW.Write, RWTableTag.Stack, 1, 1023, 0, c, 0, 0, 0), | ||
| ] | ||
| RWDictionary(9) |
There was a problem hiding this comment.
This looks very nice! I really like this RWDictionary class, makes tests more readable and less error prone!
icemelon
left a comment
icemelon
left a comment
There was a problem hiding this comment.
just a nitpick, otherwise this PR looks great
|
Could we prioritize the merge of this PR first as it's quite huge and will conflict with many open PRs? And it also introduces useful tools like |
4 participants
This PR aims to add mypy for more strict type checking.
Following @ed255's suggestion in this comment, it introduces an interface
Expressionproviding functionexprwhich returns aFQ. In this way, we are always comparing number in field, and can avoid unsupported types comparing or integer underflow issues.It's built upon #110 because #110 removes some unused things, then we don't bother to fix typing of them.
Co-authored-by: @ChihChengLiang
Co-authored-by: @ed255