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

Conditionally Run Notebooks #65

Merged
merged 17 commits into from
Jan 16, 2024

Conversation

Zeitsperre
Copy link
Collaborator

@Zeitsperre Zeitsperre commented Dec 24, 2023

Pull Request Checklist:

  • This PR addresses an already opened issue (for bug fixes / features)
    • This PR fixes #xyz
  • (If applicable) Documentation has been added / updated (for bug fixes / features).
  • (If applicable) Tests have been added.
  • CHANGES.rst has been updated (with summary of main changes).
    • Link to issue (:issue:number) and pull request (:pull:number) has been added.

What kind of change does this PR introduce?

  • Added a workflow configuration using label_on_approval.yml and modifications of main.yml so that fewer tests are run on Pull Requests before they are fully approved.
  • Added some pre-commit configurations to both clean up the code within notebooks (NbQA) and strip their outputs (nbstripout). This is removing a lot of lines from the code base.
  • tox is now fully v4.0-compliant.
  • Added a Makefile recipe to facilitate installation of esmpy when esmf is installed and visible on the $PATH.
  • Added a Makefile recipe for running tests over Jupyter notebooks.
  • Synchronized dependencies between pyproject.toml and conda configuration files.

Does this PR introduce a breaking change?

Changes to the CI? Yes. The testing suite will only be fully run when a review has been approved. This will launch a small workflow that labels the Pull Request as "Approved" and re-runs the full testing suite. All subsequent commits pushed to the branch will trigger the full test suite as well.

Changes to the library? Not really. Dependencies are in sync.

Other information:

This is nearly an identical adaptation to the workflow system I developed in xclim:

The label_on_approval.yml workflow has some specific logic to deal with pull requests that come from forks of xhydro. This is because people who fork the repo have limited privileges for security reasons.

…, add nbstripout for removing notebook outputs, update tox.ini to be fully v4.0+ compliant
@Zeitsperre Zeitsperre added the documentation Improvements or additions to documentation label Dec 24, 2023
@Zeitsperre Zeitsperre self-assigned this Dec 24, 2023
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@Zeitsperre Zeitsperre marked this pull request as ready for review January 12, 2024 20:26
@github-actions github-actions bot added the approved Approved for additional tests label Jan 12, 2024
@RondeauG
Copy link
Collaborator

Label approved added with my review and the tests got launched. Looks good!

# Conflicts:
#	.pre-commit-config.yaml
#	docs/notebooks/gis.ipynb
#	docs/notebooks/local_frequency_analysis.ipynb
#	environment-dev.yml
@Zeitsperre Zeitsperre merged commit 456c425 into basin-delineation Jan 16, 2024
16 checks passed
@Zeitsperre Zeitsperre deleted the conditional-notebook-builds branch January 16, 2024 19:36
sebastienlanglois added a commit that referenced this pull request Mar 12, 2024
…ilds (#61)

<!-- Please ensure the PR fulfills the following requirements! -->
<!-- If this is your first PR, make sure to add your details to the
AUTHORS.rst! -->
### Pull Request Checklist:
- [x] This PR addresses an already opened issue (for bug fixes /
features)
  - This PR fixes #60 
- [x] (If applicable) Documentation has been added / updated (for bug
fixes / features).
- [X] (If applicable) Tests have been added.
- [X] CHANGES.rst has been updated (with summary of main changes).
- [X] Link to issue (:issue:`number`) and pull request (:pull:`number`)
has been added.

### What kind of change does this PR introduce?
This PR adds a GIS module for usual geospatial operations that are
common in hydrology such a watershed delineation, watershed properties
extraction, etc. It adapts the work that's been done in
[ravenpy](https://ravenpy.readthedocs.io/) while also adding some new
functionalities.

**Watershed Delineation**

- [x] Support concurrent delineation of multiple watersheds
simultaneously.
- [X] Enable access to official watershed polygons
(shapefiles/geojson/geoparquet) from authoritative sources (DEH, HYDAT,
USGS, HQ, etc.) —implemented collaboratively with xdatasets.

**Physiographic Variable (or others) Extraction**

- [x] Support simultaneous extraction of physiographic variables across
multiple watersheds.
- [X] Facilitate the extraction of variables present in STAC catalogs
(e.g., Planetary Computer).
- [X] Implement extraction considering pixel weighting rather than an
"all_touched" approach, as this can significantly impact final results
—implemented collaboratively with xdatasets.

### Does this PR introduce a breaking change?
No

### Other information:
This PR also integrates the changes from #65 and #68
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Approved for additional tests documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants