-
Notifications
You must be signed in to change notification settings - Fork 9
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
test script for examples #151
Conversation
Release v0.1.0
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Check cfg
Added temperature_in_C
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small changes in workflow
.github/workflows/run_merge_test.yml
Outdated
on: | ||
push: | ||
branches: | ||
- master |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we have no master branch, I think you mean main? :)
It should probably also run on PRs like this one?
(see here for example
pull_request: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, I read up a bit and I updated the workflow that we have instead.
.github/workflows/run_merge_test.yml
Outdated
- name: Run the test examples | ||
shell: bash -l {0} | ||
run: | | ||
pip install seaborn=0.12, mpi4py, pytorch, scikit-learn, rasterio, scikit-image |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think probably you would want to set up a conda env so it works the same as our other tests (otherwise we end up maintaining two different types of workflows). miniconda install should also be more uptodate and stable I hope
See here
PASEOS/.github/workflows/run_tests.yml
Line 40 in a8126c7
- name: provision-with-micromamba |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, using the same environment as the tests now.
Update README.md to show downloads
…est-for-notebooks
closed as inactive for now |
Description
Summary of changes
Resolved Issues
How Has This Been Tested?
The script was run on my laptop. Verified by introducing errors in the examples and running the correct one.
The workflow will be tested automatically.
Related Pull Requests