Closed dnerini closed 3 years ago
The black test is failing, but I suggest we reformat the code separately once all the remaining pull requests are merged, right before releasing v1.4,
Did the tests fail, when you tried python 3.9?
Merging #187 (e9c9078) into master (753c6b3) will increase coverage by
0.32%
. The diff coverage is77.51%
.
@@ Coverage Diff @@
## master #187 +/- ##
==========================================
+ Coverage 74.40% 74.72% +0.32%
==========================================
Files 110 110
Lines 8805 8763 -42
==========================================
- Hits 6551 6548 -3
+ Misses 2254 2215 -39
Flag | Coverage Δ | |
---|---|---|
unit_tests | 74.72% <77.51%> (?) |
Flags with carried forward coverage won't be shown. Click here to find out more.
Impacted Files | Coverage Δ | |
---|---|---|
pysteps/utils/interface.py | 87.30% <ø> (ø) |
|
pysteps/visualization/animations.py | 5.68% <10.00%> (+1.08%) |
:arrow_up: |
pysteps/tests/test_plt_precipfields.py | 88.88% <50.00%> (-0.40%) |
:arrow_down: |
pysteps/visualization/utils.py | 51.61% <63.63%> (+7.64%) |
:arrow_up: |
pysteps/visualization/precipfields.py | 66.29% <66.66%> (-2.52%) |
:arrow_down: |
pysteps/visualization/motionfields.py | 71.66% <69.23%> (-0.99%) |
:arrow_down: |
pysteps/tests/test_plt_cartopy.py | 84.21% <83.33%> (ø) |
|
pysteps/visualization/basemaps.py | 81.81% <83.33%> (+32.39%) |
:arrow_up: |
pysteps/datasets.py | 38.32% <100.00%> (ø) |
|
pysteps/nowcasts/sseps.py | 87.52% <100.00%> (ø) |
|
... and 10 more |
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 87fc08f...e9c9078. Read the comment docs.
Did the tests fail, when you tried python 3.9?
For py39 there were conflicts during the creation of the conda environment and the tests failed. I think that maybe we should wait a little bit until the packages dependencies are updated in conda-forge for the windows build.
Once we can build the conda environment in windows, we could replace 3.8 with 3.9. In that way we test the minimum required python version and the latest version.
Great work on this short notice, @dnerini and @aperezhortal! Looks like we only have to make some code adjustments (later) to successfully pass the Black test, right?
The Black tests were added to check that the code base is compliant with the latest black version. Black is changing quite frequently so this can be used to indicate that we need to update black in our local development environment. Once we merge this PR, we can run black and push the changes to the main branch directly.
Did the tests fail, when you tried python 3.9?
For py39 there were conflicts during the creation of the conda environment and the tests failed. I think that maybe we should wait a little bit until the packages dependencies are updated in conda-forge for the windows build.
Once we can build the conda environment in windows, we could replace 3.8 with 3.9. In that way we test the minimum required python version and the latest version.
Agreed. Btw, we have the same issue with the PR on our conda-forge pysteps-feedstock: https://github.com/conda-forge/pysteps-feedstock/pull/11
should we add a pylint workflow?
I was thinking about this too. Now we have several pep8 on the codebase and pylint gives a very detailed report on the code (that means, it will point out several violations). I think that we can postpone the pylint checks for a little while and perhaps start with some flake8 checks on future PRs. Adding and fixing one or a few types of errors at each PR.
Once the codebase passes many flake8 tests, we can certainly add pylint.
Beginning of November 2020, Travis CI introduced a new pricing model which in practice stopped providing unlimited hours to open source projects. In only few weeks, pysteps ran out of its allocated hours (which are not renewable). For this reason, we have decided to migrate to GitHub Actions, since this still offers unlimited hours to public repositories.
In short:
TODO/questions:
- [ ] should we add a pylint workflow?