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 GitHub actions for unit testing #745

Merged
merged 68 commits into from
Sep 20, 2023
Merged

Use GitHub actions for unit testing #745

merged 68 commits into from
Sep 20, 2023

Conversation

peanutfun
Copy link
Member

@peanutfun peanutfun commented Jul 4, 2023

Changes proposed in this PR:

  • Define allowed Python versions in setup.py and environment file
  • Allow Python 3.10 and 3.11 for Climada
  • Add a GitHub workflow for CI
    • Build new conda environments for every branch. Environments are cached over one calendar day, meaning that they do not have to be rebuilt during that period if the environment files don't change.
    • Test CLIMADA with Python 3.9, 3.10, 3.11
    • Run unit tests and report test results
    • Upload coverage reports as artifacts
  • Fix some tests
    • Two nightlight tests are actually integration tests, as they require that some files have been downloaded
    • Adjust accuracy on some tests which fail otherwise
    • Use default_factory to initialize mutable default values for dataclass

PR Author Checklist

PR Reviewer Checklist

This reverts commit 9be8632.

The command does not actually use multiprocessing but makes coverage
aware of multiprocessing used by the called tests.
* Move file checks to integration tests. They require file downloads.
* Fix assertion for almost equal arrays.
The unsequa module throws segmentation faults while unit testing.
@peanutfun peanutfun marked this pull request as ready for review August 17, 2023 12:51
@peanutfun
Copy link
Member Author

I tried to achieve compatibility also with Python 3.11, but the unsequa module causes segmentation faults there, see https://github.com/CLIMADA-project/climada_python/actions/runs/5890648735/job/15976187422. I think this is due to the process pool. Maybe #763 will resolve these issues, but we will have to look out.

@chahank
Copy link
Member

chahank commented Aug 17, 2023

The build in 3.10 also fails, but because of another module. Can this be easily fixed?

@peanutfun
Copy link
Member Author

All checks have passed for the latest commit. What are you referring to? 🤔

@chahank
Copy link
Member

chahank commented Aug 17, 2023

@peanutfun
Copy link
Member Author

This was the test result of an earlier commit. The particular error was resolved by 109d994

strategy:
fail-fast: false
matrix:
python-version: ["3.9", "3.10"]
Copy link
Member

Choose a reason for hiding this comment

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

Is this something we then have to update manually for each release?

Copy link
Member Author

Choose a reason for hiding this comment

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

For each Python release, yes.

@chahank
Copy link
Member

chahank commented Aug 18, 2023

Looks good to me if this does what it should do which I cannot tell from looking at the code only. Maybe @emanuel-schmid can confirm?

@peanutfun
Copy link
Member Author

@chahank @emanuel-schmid How and where should I document this? (topics: What does the pipeline do, how to inspect results and coverage reports, etc)

@peanutfun
Copy link
Member Author

I tried to achieve compatibility also with Python 3.11, but the unsequa module causes segmentation faults there, see https://github.com/CLIMADA-project/climada_python/actions/runs/5890648735/job/15976187422. I think this is due to the process pool.

This apparently was resolved by #763. Excellent work, @chahank!

@peanutfun
Copy link
Member Author

@chahank @emanuel-schmid How and where should I document this? (topics: What does the pipeline do, how to inspect results and coverage reports, etc)

I would highly appreciate your comments on this. Apart from that, the branch is ready to be merged.

@emanuel-schmid
Copy link
Collaborator

Well - there is https://github.com/CLIMADA-project/climada_python/blob/main/doc/guide/Guide_Continuous_Integration_and_Testing.ipynb where it wouldn't be completely out of place.
But atm I'd suggest to keep it basic - let's see whether it's sustainable before making a full-fledged documentation about it.

@peanutfun
Copy link
Member Author

From personal discussion: I'll add a documentation of the pipeline itself but not make it part of the guides for testing yet.

* Add introduction to GitHub Actions to the docs.
* Add information to the CI guide.
* Improve comments in the ci.yml workflow definition.
@emanuel-schmid emanuel-schmid merged commit d8eaf55 into develop Sep 20, 2023
10 checks passed
@emanuel-schmid emanuel-schmid deleted the github-actions branch September 20, 2023 11:16
@emanuel-schmid emanuel-schmid restored the github-actions branch July 18, 2024 08:26
@emanuel-schmid emanuel-schmid deleted the github-actions branch July 18, 2024 08:26
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