Fix and narrow repr.py typing #1616
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1616 +/- ##
=======================================
Coverage 99.69% 99.69%
=======================================
Files 71 71
Lines 6860 6864 +4
=======================================
+ Hits 6839 6843 +4
Misses 21 21
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
GBeauregard
force-pushed
the
repr-changes
branch
7 times, most recently
from
02b9857 to
e2a4bbe
Compare
|
This one should be ready for review now; I'll try to slow down the pull requests to give you a chance to review them and I'll try to see what can be combined in the future. |
GBeauregard
force-pushed
the
repr-changes
branch
from
e2a4bbe to
2777568
Compare
First of many stabs at eliminating Any and # type: ignore from the codebase. Here we eliminated several different type ignores; the remaining # type: ignore is just a mypy limitation. I also narrowed the typing of RichReprResult from Iterable[Any|...]] to Iterable[object] since after inspection of the code this is what's really required. There's one particularly nasty cast that's needed. It's definitely a mypy bug that it's not throwing an error for us here. See: python/mypy#1178 and I've opened a similar issue for pyright. There's an open PR for mypy that would resolve this, but at least until this is fixed we need to be casting. In order to correctly type the auto_repr function, I introduced a SupportsRichRepr protocol; we could also have made this runtimecheckable and replaced a hasattr check in the code with it, but it seemed unnecessary so I opted not to.
GBeauregard
force-pushed
the
repr-changes
branch
from
2777568 to
e6fec11
Compare
|
Note the complicated cast I added to correctly type the case where type checkers can't handle len() narrowing. It reveals a probably unintended behavior of the code; It looks to assume the first tuple argument is I've chosen to avoid any functional changes in this PR so you can decide if you want to fix the unhandled edge cases later if you want. The current code doesn't fail anywhere; it just causes unexpected behavior. An alternate approach to the typing problem that obviates the need for the complex cast is to change the typing of Yet another approach is if we want to change the behavior we can make a |
Type of changes
Checklist
Description
Managed to squash a lot of
# type: ignorein this one (8) and every case ofAny. This was generally speaking just by fixing the typing.First of many stabs at eliminating Any and # type: ignore from the codebase.
In order to correctly type the auto_repr function, I introduced a
SupportsRichRepr protocol; we could also have made this runtimecheckable
and replaced a hasattr check in the code with it, but it seemed
unnecessary so I opted not to.
There's a bit of a surprising cast in there. There's a bug/limitation in mypy that means it's internally incorrectly using Any for the tuple unpacking there so it doesn't reveal the need for the cast, but pyright does. The cast is needed at least until len() is recognized as a type narrowing pattern.
Apologies for all the separate pull requests; I kinda want to separate out the more complicated tasks.
There is one remaining
#type: ignorein this file; it is because mypy doesn't allow inspecting__init__under any circumstance, even though our use is not buggy: https://mypy-play.net/?mypy=latest&python=3.10&gist=c422a6478274d762a9bcc049051b8304. I'm unsure if I want to bring this up with mypy devs so I haven't for now (feel free to do so yourself and link me an issue if you like). It seems to me this lint is just intended to be something they always disallow.