search

NNPDF / eko

Evolution Kernel Operators
https://eko.readthedocs.io
GNU General Public License v3.0
7 stars 2 forks source link

Introduce types in the whole package #181

Open alecandido opened 1 year ago

alecandido commented 1 year ago

At the moment, type hints has been mainly introduced in eko.io, and inconsistently here and there in other places.

We should consistently define and use types, as a starting point we should better distribute types that are common to the whole package, lifting a huge part of eko.io.types to eko.types.

This will be also helpful for #178

First discussed in https://github.com/NNPDF/eko/pull/172#discussion_r1048512758

alecandido commented 1 year ago

Despite being user-facing, and somehow part of the interface, types are optional in Python. I'll consider them as a non-breaking change, at least for the time being.

felixhekhorn commented 1 year ago

Out of the suggested tools here https://mypy.readthedocs.io/en/stable/existing_code.html#automate-annotation-of-legacy-code only MonkeyType seems a realistic option

alecandido commented 1 year ago

Do not use any automated option: types are useful to add information, while, if the information is already there, you could leave it untyped. This is perfectly fine for Python (to have types only somewhere), since Any is implicitly added.

I don't remember what's the default (I could quickly look up), but Mypy for sure has an option to (dis)allow Anys.

So, let's keep doing it incrementally. If you want to add a bunch of them manually, feel free to do it. But even in that case, please break it in multiple PRs.