Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use type hinting #674

Merged
merged 13 commits into from
Jan 5, 2023
Merged

use type hinting #674

merged 13 commits into from
Jan 5, 2023

Conversation

12rambau
Copy link
Member

@12rambau 12rambau commented Dec 21, 2022

#612

  • use type hinting in all files
  • create a nox mypy session (pre-commit is not working very well)
  • use type hints in the documentation

the mypy support is very very complicated as everything needs to be typed to not raise an error. On my local machine there are still 237 errors to take care of but I think it's not very meaningful to adress them all here. I would prefer to run it once in a while and reduce them step by step.

Using pre-commit was not possible as the imports are not available without install and pre-commit don't do that: https://github.com/pre-commit/mirrors-mypy#mypy-mirror

I tried my best to keep the previously set values (None, "", False) and the tests are not complaining so it seems ok.

Note that it's impossible to do something like:

class A:
    a: int

   def __init__(self):
       self.a = 5

And document it as a is not set before the call to __init__. That's why I decided to keep all the Noneinitialized variables and set them to Optional[xx] type.

For readability, I moved the definition of a class into the __init__ method to get the hint in the parameters. I don't know yet if I keep the public constructor in the doc or not though

  • refactor all methods
  • use the type hints in the doc
  • use the type hint in the precommit (mypy)

edit
Cannot go any further as mypy is not supporting Self hint yet. python/mypy#14041 will be merged in v0.992

@codecov
Copy link

codecov bot commented Dec 21, 2022

Codecov Report

Merging #674 (6197fd4) into main (49f8a5a) will increase coverage by 0.67%.
The diff coverage is 99.47%.

@@            Coverage Diff             @@
##             main     #674      +/-   ##
==========================================
+ Coverage   92.53%   93.21%   +0.67%     
==========================================
  Files          38       39       +1     
  Lines        3927     4330     +403     
==========================================
+ Hits         3634     4036     +402     
- Misses        293      294       +1     
Impacted Files Coverage Δ
sepal_ui/mapping/sepal_map.py 90.27% <90.24%> (+0.05%) ⬆️
sepal_ui/mapping/legend_control.py 93.65% <95.23%> (-1.09%) ⬇️
sepal_ui/planetapi/planet_model.py 96.51% <96.42%> (+1.00%) ⬆️
sepal_ui/aoi/aoi_model.py 89.62% <100.00%> (+0.31%) ⬆️
sepal_ui/aoi/aoi_tile.py 100.00% <100.00%> (ø)
sepal_ui/aoi/aoi_view.py 98.87% <100.00%> (+0.05%) ⬆️
sepal_ui/frontend/styles.py 100.00% <100.00%> (ø)
sepal_ui/mapping/aoi_control.py 100.00% <100.00%> (ø)
sepal_ui/mapping/basemaps.py 96.66% <100.00%> (-0.21%) ⬇️
sepal_ui/mapping/draw_control.py 97.61% <100.00%> (+0.05%) ⬆️
... and 23 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@12rambau 12rambau merged commit bc1804f into main Jan 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant