fjosw
[Feat] Add type hints
As a first step towards adding type hints, I added automatic type hints with monkeytype. Hints for the more complicated methods will need additional manual work but for the simpler methods this seems to have worked quite well.
Not quite done but already in decent shape for a review @s-kuberski @jkuhl-uni:
- Most of the changes are just type annotations
- I changed a few lines to help mypy with understanding the type annotations
- I fixed a few smaller bugs in which explicit type checks were not consistent or did not cover certain edge cases
For now I ignored the input/* modules and most of correlators.py, the remaining modules only raise a few smaller mypy errors which I wasn't able to fix right away.
Great, thanks a lot for all of your work! I'll have a look in the upcoming week.
Hey @fjosw, thanks for all the work, I am looking through it right now, it looks excellent to me. I hope it was okay that I pushed a few small changes. If not, feel free to revert them, as you please :)
Thansk again for starting this. I'm still in correlators.py and have discovered quite a few places, where I think that the type hints should be changed, if one wants them to be correct. What's our strategy? Shall I continue to point them out and you change them if you agree? Or shall I directly adapt? I guess it's not breaking anything if some of the hints are incorrect, but if we introduce them in the first place, I think we should make an effort to have them correct and hopefully complete.
Feel free to get started with fixing stuff, you should have permissions to push to the branch, I think I won't have time to work on this in the comming days. My strategy so far was to run mypy to point out typing issues and go from there. As @jkuhl-uni suggested it might be worth defining a Obs-like type which can then be used for the overloaded methods.
I've tried to go through the changes and to address the issues that I raise above and a few more things.
I resolved the merge conflicts and fixed a few smaller issues but now one of the tests test_rwms is failing. Can you have a look @jkuhl-uni ?
I had a look and found a solution for now, there are a few minor things to figure out. I hope to get it done today :)
... well that was easy.
I'd like to disentangle some of the openQCD tests (in test_rwms we are testing the read_rwms, extract_t0 and extract_w0 methods), but I'll do that in a different PR.
Thanks a lot @jkuhl-uni ! As we have accumulated quite a few changes I would suggest that we all do a thorough review and then merge this first version with type hints which we can refine later @s-kuberski @jkuhl-uni .
Hi, I looked through the code again and made mainly small changes, as to improve some type hints. The PR looks good to me otherwise.