search

ComPWA / expertsystem

Rule based particle reaction problem solver on a quantum number level
http://expertsystem.rtfd.io
1 stars 3 forks source link

Add type hints #43

Closed redeboer closed 3 years ago

redeboer commented 4 years ago

It turns out to be quite difficult to refactor the codebase. As a first step, it may therefore be smart to add type hints to all functions and variables, because they reveal intention of the functions. It then becomes more easy to refactor them.

Mypy allows to exclude certain modules with the tox.ini config, as happens in #42. I propose incrementally removing the excludes per module and adding type hints, starting from the top of the dependency tree. Created on cf1728eedf466f2d590a6eee77ddfb2b94359b76 with pydeps:

pydeps expertsystem -T png

See also: https://github.com/ComPWA/expertsystem/blob/d4f1928ccaaa208f36f428d5dfdb8a94fa75fbbc/tox.ini#L42-L44

[Estimated effort: 1-3 days]

spflueger commented 4 years ago

I'm not sure how much this will help for understanding the design. I added an estimated effort for this issue, which I believe to be a few days. My feeling says its just going to eat up a few days of time and then understanding will be similar. It would not be wasted time since those type hints should be implemented eventually. But they are low priority to me.

redeboer commented 4 years ago

For background: I already started doing this in practice, because I don't know the code-base well enough (there are several implicit assumptions about the input dicts) and needed some way to make notes. I say in practice, because I tried refactoring right away which turned into a time-consuming debugging session and I saw that this work would turn into a PR that is impossible to review. Then I realised this work could (should) be split into an issue to keep the eventual PR separate from the eventual refactoring work.