sopel-irc / sopel

:robot::speech_balloon: An easy-to-use and highly extensible IRC Bot framework. Formerly Willie.
https://sopel.chat
Other
949 stars 403 forks source link

Clean up errors in `mypy --check-untyped-defs` #2471

Closed dgw closed 1 year ago

dgw commented 1 year ago

Description

Made in collaboration with @SnoopJ, this patch resolves about 70 type-checking errors between the two of us. #2462 took care of errors in typed defs, and this PR takes care of the untyped ones.

Along the way, we also confirmed that upgrading to mypy 1.3 works without any new errors (it actually fixed a couple that were due to bugs in older mypy releases), and as the final commit this patch also changes the default behavior of make mypy to include --check-untyped-defs.

Specifically requesting @Exirel's review as our resident typing expert, rather than just any rockstar (sorry y'all!).

Note: There are two unfixed errors in sopel/modules/remind.py if only this patch is applied. They're fixed separately in #2470 (which is a bugfix, rather than housekeeping, patch.)

Checklist

Exirel commented 1 year ago

I only want to rebase this monster once…

Should be pretty close now.

dgw commented 1 year ago

Just marked the last thread (that isn't a reminder for the upcoming rebase/squash) as resolved. Big re-review request for @Exirel now put in. 😁 (I heartily recommend using the "Show changes since your last review" tool for this one, at least to start. You can still re-review the whole patch afterward if you're happy with the individual resolutions.)

dgw commented 1 year ago

Squash done. Unusually (for me), the force-push diff is not a no-op, because in addition to squashing fixups & dropping commits that were later reverted, I also dropped the last three (converting some annotations to pull from typing) because they're going to get immediately undone anyway in my next WIP cleanup branch.

Just worked with @SnoopJ on IRC to split/combine a few more commits and fix broken intermediate states where the existing checks with pytest/flake8 fail. Now running a check through the rest of the branch in case there's another broken intermediate commit that I missed in the first check, before force-pushing one last time.

dgw commented 1 year ago

And, it's done! 49 commits & oopsies 😉 squashed down to 31 logical (for the most part?) steps, with—according to the checks I ran on each commit during the most recent rebase—no test-failing intermediate stages! Both pytest and flake8 passed for every one of the 31 steps it took to get here. 🥳