DeltaRCM / pyDeltaRCM

Delta model with a reduced-complexity approach
https://deltarcm.org/pyDeltaRCM/
MIT License
18 stars 10 forks source link

Add additional type annotations #253

Closed r-barnes closed 2 years ago

codecov[bot] commented 2 years ago

Codecov Report

Merging #253 (0418d2e) into develop (a5b72b5) will decrease coverage by 0.03%. The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop     #253      +/-   ##
===========================================
- Coverage    79.01%   78.98%   -0.04%     
===========================================
  Files           12       12              
  Lines         2612     2608       -4     
===========================================
- Hits          2064     2060       -4     
  Misses         548      548              
Impacted Files Coverage Δ
pyDeltaRCM/__init__.py 100.00% <100.00%> (ø)
pyDeltaRCM/debug_tools.py 79.41% <100.00%> (ø)
pyDeltaRCM/hook_tools.py 98.30% <100.00%> (ø)
pyDeltaRCM/init_tools.py 97.15% <100.00%> (-0.01%) :arrow_down:
pyDeltaRCM/iteration_tools.py 98.00% <100.00%> (-0.02%) :arrow_down:
pyDeltaRCM/model.py 89.44% <100.00%> (-0.02%) :arrow_down:
pyDeltaRCM/preprocessor.py 84.09% <100.00%> (-0.04%) :arrow_down:
pyDeltaRCM/sed_tools.py 44.48% <100.00%> (ø)
pyDeltaRCM/shared_tools.py 76.41% <100.00%> (ø)
pyDeltaRCM/water_tools.py 54.54% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update a5b72b5...0418d2e. Read the comment docs.

amoodie commented 2 years ago

Hi Richard, thanks for checking things out here and trying to clean up the code base.

I have to ask though: are all these type checks necessary? I scanned through quickly and found a few type annotations in the model that are wrong, so I'm wondering if they were auto generated or you did them manually? I guess I will need to check them carefully before merging. What happens if the wrong type is passed to the method; does Python care or are these just for static code analyzers? I guess it doesn't hurt to have them if it is just the latter?

r-barnes commented 2 years ago

@amoodie - Apologies. The first PR I did by hand. This one was semi-automated.

The type annotations have no run-time effect, but can be interpreted by IDEs (such as vscode+pylance) for real-time type consistency checking and used with static analyzers (Pyre, mypy, pylance) to test for consistency errors without having to run a test suite.

They are not strictly necessary, but there are some advantages to having them:

To be clear, the main advantage for me is that these allow me to reason about what's happening in the code.