search

nebari-dev / nebari

🪴 Nebari - your open source data science platform
https://nebari.dev
BSD 3-Clause "New" or "Revised" License
274 stars 88 forks source link

[ENH] - Add a static type checker? #1800

Open pmeier opened 1 year ago

pmeier commented 1 year ago

Feature description

Add a static type checker like mypy to our lint workflows. Note that this doesn't mean that we have to declare support for PEP561. I mostly want to make sure that the type hints that we have in our source are correct.

Value and/or benefit

For me personally the following holds:

correct type hints > no type hints > wrong type hints

I'm aware that type hints are not everyone's cup of tea and they prefer no type hints, but I don't think anyone can argue that wrong type hints aren't detrimental for everyone.

We currently have type hints in a few places, but no way to enforce their correctness. One example of wrong type hints is

https://github.com/nebari-dev/nebari/blob/8bd56f90b84c7686f45459ae8f7cf690721bd60a/nebari/render.py#L229-L231

https://github.com/nebari-dev/nebari/blob/8bd56f90b84c7686f45459ae8f7cf690721bd60a/nebari/render.py#L77-L81

The type hint has to be List[str] in both cases.

Thus, if we want to use type hints, and I'm all for that, we should also have a checker that enforces their correctness.

Anything else?

If we decide that we don't want to have a type checker, we should also purge all type hints from the repository and enforce that no new ones are added. Sounds harsh, but I don't think there is another option to avoid wrong ones.

iameskild commented 11 months ago

This was discussed during today's community call and we decided that we would incrementally move towards fixing all of the type annotations. We will add mypy to CI but for the time being ignore failures. @fangchenli has already started on this effort and has raised several PRs 🙌

cc @pavithraes @dcmcand @fangchenli