jszopi / repESP

Reproducing molecular ESP from partial charges and more
GNU General Public License v3.0
6 stars 7 forks source link

mypy: Discuss the impact of no `--disallow-any-expr` #44

Closed jszopi closed 5 years ago

jszopi commented 5 years ago

A single Any type can seriously undermine the benefit from having a type checker. I've just spent quite a lot of time on getting the codebase to a strict mypy type check level, so I don't feel like working on the 50+ errors that appear with the --disallow-any-expr flag, but they could potentially hide severe type errors. A compromising alternative would be to monitor the output of --any-exprs-report in CI, but unless that integrates nicely with coverall, it won't be useful. The brief report is currently:

                      Name   Anys   Exprs   Coverage
----------------------------------------------------
              repESP._util      0      66    100.00%
        repESP.calc_fields     18     110     83.64%
        repESP.charge_util      3      86     96.51%
            repESP.charges      9      87     89.66%
        repESP.cube_format      0     346    100.00%
        repESP.equivalence     11     137     91.97%
           repESP.esp_util     15     524     97.14%
             repESP.fields      8     334     97.60%
    repESP.gaussian_format      0     317    100.00%
repESP.resp_charges_format     11      40     72.50%
       repESP.resp_wrapper      2     297     99.33%
      repESP.respin_format     24     682     96.48%
  repESP.respin_generation      0     241    100.00%
              repESP.types      9     142     93.66%
               repESP.util      0      48    100.00%
----------------------------------------------------
                     Total    110    3457     96.82%

Note that this concern applies to user scripts as well. As soon as the user creates an annotated function to calculate a parameter for passing to a library function, mypy would deduce Any for the type. This particular scenario is alleviated by having the user use the --strict flag, which implies --disallow-untyped-defs, but users may then get strongly discouraged by the type errors and start ignoring mypy. They can still use explicit Any too.

jszopi commented 5 years ago

Type-based runtime checks may be the only way of making sure that users don't misuse the library without them embracing mypy to the max. It can also catch some errors due to the library not having a 100% Any coverage. One library that does runtime checks is pydantic, but I haven't recently researched the shifting landscape.

jszopi commented 5 years ago

Closing in favour of runtime type checking in #48.