alan-turing-institute / DTBase

A starting point from which digital twins can be developed.
MIT License
11 stars 4 forks source link

Chore/95 add python type annotations #127

Closed GiorgioCerro closed 11 months ago

GiorgioCerro commented 11 months ago

This is part of the type annotation. As Markus suggested I can make this pull request in the meantime so we can check whether variables are aligned and keep working on what is left, which is models and core.

review-notebook-app[bot] commented 11 months ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

codecov-commenter commented 11 months ago

Codecov Report

Merging #127 (017c04a) into main (8d4834a) will increase coverage by 0.16%. Report is 2 commits behind head on main. The diff coverage is 100.00%.

:exclamation: Current head 017c04a differs from pull request most recent head 964843d. Consider uploading reports for the commit 964843d to get more accurate results

@@            Coverage Diff             @@
##             main     #127      +/-   ##
==========================================
+ Coverage   86.04%   86.20%   +0.16%     
==========================================
  Files          54       54              
  Lines        4035     4075      +40     
==========================================
+ Hits         3472     3513      +41     
+ Misses        563      562       -1     
Files Coverage Δ
dtbase/backend/api/__init__.py 85.71% <100.00%> (+0.25%) :arrow_up:
dtbase/backend/api/location/routes.py 82.45% <100.00%> (+0.15%) :arrow_up:
dtbase/backend/api/model/routes.py 84.21% <100.00%> (+0.11%) :arrow_up:
dtbase/backend/api/sensor/routes.py 85.71% <100.00%> (+0.10%) :arrow_up:
dtbase/backend/utils.py 88.88% <100.00%> (+4.27%) :arrow_up:
dtbase/tests/conftest.py 86.56% <100.00%> (+0.62%) :arrow_up:
dtbase/tests/generate_synthetic_data.py 65.47% <100.00%> (+0.84%) :arrow_up:
dtbase/tests/test_api_locations.py 100.00% <100.00%> (ø)
dtbase/tests/test_api_models.py 100.00% <100.00%> (ø)
dtbase/tests/test_api_sensors.py 100.00% <100.00%> (ø)
... and 18 more

... and 1 file with indirect coverage changes

EdwinB12 commented 11 months ago

Okay, great that tests are passing. @mhauru - I'm just trying to understand your commits in this PR. Are these extra linting changes?

mhauru commented 11 months ago

@EdwinB12, those were fixing issues that came up with Windows vs Unix stuff. Having edited the code on Windows, Giorgio's commits had changed all the line endings to be a different control character than the Unix standard, and similarly changed the file permissions because permissions on NFTS work differently from Unix filesystems. So I changed those back, and Giorgio changed some git settings so that in the future his git shouldn't commit those sorts of changes. I think similarly for the one symlink, presumably because symlinks on Windows are different that one had somehow been changed, so I reverted that.

The one thing I don't understand is that locally it seemed to me like this PR deleted the Pulumi.test.yaml file, so I restored it, but now in the diff it seems like this PR is adding that file. I think I messed that up somehow, I'll take a look.

mhauru commented 11 months ago

I usually do a regular merge, but once we are ready to merge this one, maybe we should do a squash-merge. Otherwise there will first be a commit that changes every single line in the repo (because the line end character has changed) and then another commit that does that again to revert the previous change. That would we make any sort of git blame messy, so better to squash all that into a single commit that only shows the total changes coming from this PR.

EdwinB12 commented 11 months ago

I was going to suggest exactly this

EdwinB12 commented 11 months ago

I'll approve. Feel free to merge Markus once you're happy.