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

Migrate to GitHub Actions #187

Merged
merged 52 commits into from
Nov 24, 2020
Merged

Migrate to GitHub Actions #187

merged 52 commits into from
Nov 24, 2020

Conversation

dnerini
Copy link
Member

@dnerini dnerini commented Nov 22, 2020

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:

  • test for linux-latest, macos-latest and windows-latest (note that the latter was not tested under Travis CI)
  • test for Python 3.6 and 3.8 (the compromise strategy is to test for the oldest and latest supported python versions only)
  • Black check runs on a separate workflow

TODO/questions:

  • code coverage on Codecov
    - [ ] should we add a pylint workflow?

@dnerini dnerini changed the title Create main.yml Github actions Nov 22, 2020
@dnerini dnerini marked this pull request as draft November 22, 2020 18:28
@codecov
Copy link

codecov bot commented Nov 24, 2020

Codecov Report

Merging #187 (e9c9078) into master (753c6b3) will increase coverage by 0.32%.
The diff coverage is 77.51%.

Impacted file tree graph

@@            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%) ⬆️
pysteps/tests/test_plt_precipfields.py 88.88% <50.00%> (-0.40%) ⬇️
pysteps/visualization/utils.py 51.61% <63.63%> (+7.64%) ⬆️
pysteps/visualization/precipfields.py 66.29% <66.66%> (-2.52%) ⬇️
pysteps/visualization/motionfields.py 71.66% <69.23%> (-0.99%) ⬇️
pysteps/tests/test_plt_cartopy.py 84.21% <83.33%> (ø)
pysteps/visualization/basemaps.py 81.81% <83.33%> (+32.39%) ⬆️
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.

@dnerini dnerini added this to the release v1.4 milestone Nov 24, 2020
Copy link
Contributor

@RubenImhoff RubenImhoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@aperezhortal
Copy link
Member

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.

@aperezhortal
Copy link
Member

aperezhortal commented Nov 24, 2020

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.

@dnerini
Copy link
Member Author

dnerini commented Nov 24, 2020

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: conda-forge/pysteps-feedstock#11

@aperezhortal
Copy link
Member

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.

@dnerini dnerini merged commit 221a28f into master Nov 24, 2020
@aperezhortal aperezhortal deleted the gh-actions branch August 28, 2022 10:20
@aperezhortal aperezhortal restored the gh-actions branch August 28, 2022 10:20
@aperezhortal aperezhortal deleted the gh-actions branch August 28, 2022 10:21
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.

3 participants