Enforce type checking in tests and CI

[fpagnoux]

Extracted from #871

Typing remains optional: the type checker will only make sure that the optional annotations are consistent with the code.

Why typing ?

• Useful as documentation: methods are easier to understand when the input and outputs type are explicit
• Allows to detect some bugs early (works better when there are more annotations in the code)
• See https://www.youtube.com/watch?v=pMgmKJyWKn8 for more 🙂

[Morendil]

I'm kind of surprised that the type annotations in simulation_builder.py are accepted by mypy, as they were written purely as documentation and reasoning aids and include things the checker wouldn't understand. What does mypy check or not check exactly?

[Morendil]

@fpagnoux Add .mypy_cache/ to .gitignore ?

[fpagnoux]

I'm kind of surprised that the type annotations in simulation_builder.py are accepted by mypy, as they were written purely as documentation and reasoning aids and include things the checker wouldn't understand. What does mypy check or not check exactly?

It looks like mypy, at least in its current version, doesn't fully enforce class attributes types annotations.

The following code doesn't raise any error:

class Test:
    def __init__(self):
        self.dict: Dict[str, int] = {}

    def get(self, key) -> str:  # inconsistent with the attribute type, should be int
        return self.dict[key]

Yet the following does:

from typing import Dict

class Test:

    def __init__(self):
        self.dict = {}

    def get_dict(self) -> Dict[str, int]:
        return self.dict

    def get(self, key) -> str:
        return self.get_dict()[key]

test.py:12: error: Incompatible return value type (got "int", expected "str")

It looks like mypy enforces better function signatures than attributes type.

However, they're not totally ignored, as the following does raise an error:

from typing import Dict

class Test:

    def __init__(self):
        self.dict: Dict[str, str] = {}

    def get_dict(self) -> Dict[str, int]:
        return self.dict

test.py:9: error: Incompatible return value type (got "Dict[str, str]", expected "Dict[str, int]")

[fpagnoux]

I've open an issue to try to understand better mypy's behaviour, and see if this is a bug likely to get resolved.

[Morendil]

See also this recent and possibly relevant article. I don't think the criticisms therein would necessarily stop me from trying out types. I feel better for knowing about this beforehand though.

[fpagnoux]

We've got a reply. It looks like you need to add -> None to init for annotations to be parsed there.

Doing so in simulation_builder raises an error at least for one annotation (str(period)) but some other exotic ones (Entity.plural) are accepted.

How should we move forward with this PR? I think I've seen/read/experimented enough to be wiling to try enforcing type checking in our codebase, but we could delay that if @sandcha and you would like more time to form an opinion.

[Morendil]

I'm OK to merge, on the understanding that it's an experimental feature; the response to an unexpected breaking build might be to disable the checking if there's no clear way to fix the type annotations. How does that sound?

[fpagnoux]

Sounds good to me!