Caltech-AMBER / ambersim

In-house tools built on GPU-accelerated simulation
MIT License
7 stars 2 forks source link

Stricter `pyright` Setup #24

Open alberthli opened 1 year ago

alberthli commented 1 year ago

Add more type checks with pyright. This is a first stab just to gut check how much of a pain it is to enforce this officially vs. just as a strong suggestion for documentation (hint: it was pretty annoying).

alberthli commented 1 year ago

@vincekurtz I realized when playing around with pyright tonight that we had really loose settings on type checking on main (not even talking about the strict setting, which is super pedantic about everything). When I adjusted this, there were a lot of errors that came up, many of them inscrutable and fairly difficult to solve. I ended up hacking the last few just to forcibly pass the pyright checks.

Here's my philosophy on this right now. Type hints are really useful, but are still just hints. I don't think we should strongly enforce typing (especially during CI), but I do think we should keep up a standard of typing things (and calling people out in review).

I also think that locally, it's not a bad idea to run pyright with slightly stricter settings because it actually does catch confusing typing, and this is useful to fix for clarity. However, more often than not, it complains about things which have basically no effect on clarity, and just kills dev time. About ~20% of the changes I made in this demonstration PR (not to be merged) were actually useful.

I put some comments on the useful examples that the stricter pyright settings caught below.