fix: enable and fix full type checking

[henryiii]

Discovered by #507; we are currently not actually enabling any of the settings it looks like we are enabling due to a missing .*. Fixes the various errors, and reworks the Colors/Symbols to be statically typeable.

Adding pytest does slow down the type checking a bit; we could revert to ignoring pytest if the pre-commit run is too slow.

I've also added a py.typed file (not sure if it's in the SDist without a manifest), but it's nice to say we are typed, even if we don't really expect to be used as a library?

Could be simplified a bit by #508.

[henryiii]

@joerick Can you see if there's a setting to enable CircleCI to build on forked PRs that's disabled, perhaps? https://circleci.com/blog/triggering-trusted-ci-jobs-on-untrusted-forks/

Edit: Nevermind, it's because I pushed a new commit.

[joerick]

I actually noticed this a couple weeks back, probably should have said something but thought I'd hold back that revelation it until things were a little quieter around here!

Thank you @henryiii for performing all these fixes. This might be a good opportunity to consider which of these 'strict' rules we actually want? Although, looking at the diff it's not too crazy, I'm mostly wondering about compulsory -> None annotations or the changes to the Colors Symbols objects.

Other that that, maybe we should define our own PathLike (os.PathLike[str]) and StrOrPath (Union[str, os.PathLike[str]]) types? I think that might clean up a lot of the existing annotations

[henryiii]

Returning -> None is useful information; it helps you not return self or something like that, it also protects you from subclassing and accidentally changing an output. But most importantly, it lets you immediately tell that a function is not supposed to return something, making the code clearer. I shouldn't have to hunt the entire body of a function to see if there's a non-None return (which I had to do to add the types, and I got it wrong on a couple of functions at first).

Changing Colors and Symbols from classes within classes that have magic accessors to simple classes doesn't seem too bad to me - thinking statically promotes simper, clearer design, I think. If we have Python 3.7, we might be able to come up with a dataclass that is simpler (but I rather doubt it would be that much simpler). We could also use a color library (I've written one before, plumbum.colorlib, but sadly it's not available standalone from plumbum, and you are using my competitor Click instead - looks like they added color support too at some point, though I'm sure plumbum.colors runs circles around it :P ).

MyPy supports gradual typing, but that's mostly so you can turn it on bit by bit, not that partially typed should be a goal. I usually run mypy --strict to see how far off something is from fully typed.

PS: The cibuildwheel part really does pass mypy --strict now, just unit_test and test fail, and you can't --strict just a portion of the check.

[henryiii]

I pulled out the types to a file, makes a nice simplification (especially since we only use PathOrStr everywhere). Dropped pytest from the pre-commit environment and allow it to be missing - it changes a 1-2 second type check into a 10 second type check or so, for basically no gain.

[joerick]

Thank you @henryiii ! I feel a little guilty for having somebody else tidy up my mess! Happy to leave things on strict settings for now.

It is interesting how typing changes what's considered 'neat' in programming practices. I'm thinking mostly about the Colors/Symbols thing. Generally I do think it's a good thing, but it does sometimes feel that one is being pushed in a more 'uniform'/'standard' direction (less 'fun'?).

Anyway... it would be good to get this in ASAP as there might be conflicts that would be more easily resolved sooner rather than later.

[henryiii]

more 'uniform'/'standard' direction (less 'fun'?)

Yes. But often, that's better long-term design, easier to read/maintain, less likely to break, easier to use downstream due to the typing. While it often can be more repetitive, you also have more checks verifying that you remembered to change everything.

There are special cases where "fun" is more important, and then you can just "Any"-it and go full dynamic. But that should probably be the point of the library, rather than just part of implementing something else.

There's still plenty to have fun with! Restricting the rules just focuses your efforts on the important parts.