Add hints for parameters and return values!

[ffl096]

TopoNetX for the most part does not use type hints for function parameters and return values, which would be usable by static type checkers such as mypy (in TopoNetX itself but also when consumers want to check their code for errors).

However, recently, some comments by @josefhoppe added a few type hints to the source code, e.g., in #94. I think there should be a clear decision whether to add type hints or not:

• If not, remove the existing ones again.
• If yes, make this clear. Even if not adding them to the existing code right away, any new code should not be merged without type hints. See also mypy's guide for existing codebases.

Either way, the decision should be made clear in the contribution guidelines and enforced for new pull requests.

[mhajij]

what is your recommendation ?

[ffl096]

I personally always use type annotations in my code, as it generally improves IDE support (suggestions, errors).

[mhajij]

ok then, I will start addressing this issue then.

[josefhoppe]

I agree. In the medium term, I think we should also add mypy to the linters as it can easily detect some type errors (I just ran it manually and it found two mistakes that can be easily overlooked by humans, even with the limited type hinting currently present in the codebase)

[josefhoppe]

(or maybe flake8 has a type checking plugin, I don't have a strong preference for a specific tool, just type checking in general)

[ffl096]

(or maybe flake8 has a type checking plugin, I don't have a strong preference for a specific tool, just type checking in general)

flake8 doesn't do much on its own, it delegates the heavy work to other existing tools. There is a plugin for mypy as well, but I think it's better to use mypy directly. That one really only exists as a workaround for editors that have integrated flake8 support but no integration for mypy.

[michaelschaub]

It appears the discussion has a relatively clear outcome (so I changed the title of the issues). It seems, clear we should do it, but it is not a top-priority right now. However, we should probably add this as requirement for new commits.

[ninamiolane]

@ffl096 do you have updates on this? Should we close it?

[ffl096]

This is an ongoing effort and will be automatically closed once #163 is merged. The remaining issues are non-trivial and will take more time.

[ninamiolane]

@ffl096 when will you expect this to be completed?

[USFCA-MSDS]

@ffl096 should we close this task ?

[ffl096]

See #163.