Add mypy

[felixhekhorn]

Closes #178

Current state is Found 63 errors in 17 files (checked 287 source files) - see CI

[felixhekhorn]

some problems (accumulated so far):

1. src/eko/io/struct.py:283: error: Incompatible return value type (got "tuple[Any, ...]", expected "tuple[float, int] | None") [return-value]: this is going through numpy and I don't know how to cast solved in 34c68a5
2. src/eko/evolution_operator/grid.py:156: error: "object" has no attribute "path" [attr-defined]: this is going through a dict and I don't know how to cast solved in 9271ebc
3. src/eko/runner/operators.py:21: error: Incompatible return value type (got "list[Operator | None]", expected "list[Operator]") [return-value] an Inventory does not know if it is contentless and so __getitem__ might return None - however in practice here this is not allowed to happen solved in 34c68a5
4. unit tests are failing, because I made num_flavs_ref mandatory, but banana does not provide it - shall we change there?

[felixhekhorn]

With this PR we introduce mypy via pre-commit. This would be a further check which developers need to control.

• The changes are mainly backward compatible, BUT (as said above) the change to num_flavs_ref which is no longer an optional parameter, but a mandatory one. This reaches beyond eko, as banana would need to provide a value - if we are happy with that change, I can open a new PR there (else here we fail unit tests). (One could consider this even a breaking change - as already said: our no-default-strategy clashes with the public interface.)
• for the most part, I did the minimal required change to make mypy running - so Introduce types in the whole package #181 still remains to be done
• for the moment the check is restricted to src/ at some point we can consider widening ...

[alecandido]

• for the most part, I did the minimal required change to make mypy running - so Introduce types in the whole package #181 still remains to be done

I'm also fine with closing #181, since Mypy will actually run. Or keeping it open, as a reminder to incrementally adopt types.

In any case, this is something that won't finish ever, since types will always be optional, no matter what (unless we ban Any in Mypy, but I still feel it's a bit too hardcore...).

[felixhekhorn]

here

eko/src/eko/io/types.py

Lines 23 to 24 in 9484666

	FlavorsNumber = int
	FlavorIndex = int

we should replace int with Literal[3,4,5,6]

[alecandido]

we should replace int with Literal[3,4,5,6]

Ah, yeah, it could be stricter, but I didn't bother in restricting too much.

However, pay attention that Literal[3,4,5,6] might be too restrictive:

from typing import Literal

def subtract(n: int, m: int) -> int:
    return n - m

flavor: Literal[3,4,5,6] = subtract(7, 2)

flags the following error:

ciao.py:8: error: Incompatible types in assignment (expression has type "int", variable has type "Literal[3, 4, 5, 6]")  [assignment]

i.e. Literal has to be a Literal, it can't be an integer with that value (the main point is that Mypy check does not happen at runtime, if you want to enforce this kind of bounds would be better to start using Pydantic).

[alecandido]

I would fix the problem with unit tests before merging, worst case ignoring them (fixing banana is definitely a better alternative).

[felixhekhorn]

@alecandido it seems pylint does not consider EKO.read type safe (see the error before) and I couldn't convince him so I needed the big hammer in a49fca6

[alecandido]

@alecandido it seems pylint does not consider EKO.read type safe (see the error before) and I couldn't convince him so I needed the big hammer in a49fca6

I'm sorry for it, but it is more of a Pylint problem, since the EKO.read() method as even an unneeded assertion just for the sake of the type (EKO.open() is the ambiguous one, which is returning a union, but the other one should be alright).

If you wish, you could also add a return hint, by using -> "EKO" (that would be typing.Self from py3.11 on).

[felixhekhorn]

Note that mypy detected the bug of missing import in 4be6d44 - so I'd say we really want this live

[felixhekhorn]

this removal is a very suspicious change ... because I introduced it in #391 (d5f63fe to be specific) and unit tests were passing - but I have to remove it here to make the unit tests pass and I can not see how the other changes here would change that ...