Skip to content

Single python script2 - #181

Merged
nhsavage merged 20 commits into
mainfrom
single_python_script2
Aug 5, 2026
Merged

Single python script2#181
nhsavage merged 20 commits into
mainfrom
single_python_script2

Conversation

@nhsavage

@nhsavage nhsavage commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

change the top level driver script to be python based

@Eleanor-Dean

Eleanor-Dean commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

This is still not working right for me -->
unless im missing the intention, the ./{base} in the config yml + the mkdir in the run_all.py are making the scratch directory in the OSOP file system.

If this is intentional thats fine but I fear it is because the pythonpath is now linked to the osop root - it makes it so when you run any code it attempts to commit the changes. Straight run with no flags caused vs code git hub to suggest commiting 627 files.

TLDR:
the outputs appear under --> osop/scripts/outputs. Causing file backlog when running

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR replaces the legacy top-level shell orchestration with a Python-based runner (scripts/run_all.py) driven by a single YAML configuration file (osop_config.yml), and updates documentation/tests accordingly.

Changes:

  • Add a Python orchestrator (scripts/run_all.py) that executes existing workflow steps via subprocesses using a YAML config.
  • Add comprehensive pytest coverage for config parsing/validation and pipeline command construction (scripts/tests/test_run_all.py).
  • Remove legacy shell runners and an unused downloader script; update user documentation (docs/source/run.md).

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
scripts/tests/test_run_all.py New unit tests for the Python orchestrator (config validation + pipeline behavior via mocking).
scripts/run_all.sh Removes the legacy bash top-level runner.
scripts/run_all.py Adds the new Python top-level orchestrator replacing the shell runner.
scripts/output/single_script/logfiles/download_log_total_precipitation_meteo_france_5_2026-07-31_09:41:26.txt Adds a captured runtime log output (likely unintended repo artifact).
scripts/get_any_hc_fc_iri.py Removes a deprecated/unused downloader script.
scripts/all_mons_precip_array.sh Removes legacy SLURM/bash array runner.
scripts/all_mons_array.sh Removes legacy SLURM/bash array runner.
osop_config.yml Adds the default YAML configuration file consumed by the new runner.
docs/source/run.md Updates “Running the toolkit” docs to the new Python + YAML entrypoint.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread scripts/run_all.py Outdated
Comment thread scripts/run_all.py Outdated
Comment thread scripts/run_all.py
Comment thread scripts/tests/test_run_all.py
Comment thread scripts/run_all.py
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown

Coverage

Coverage Report
FileStmtsMissCoverMissing
lib/osop
   compare_terciles.py76760%9–336
   compute_products_func.py1341340%8–449
   compute_scores_func.py1541540%17–414
   ens_plotting.py1425462%112–148, 240, 250, 318–337, 357–418
   plot_verify.py17314516%89–126, 158–249, 276–309, 339–434, 458–535
   pycpt_convert.py39428727%76–99, 116–122, 140, 163–169, 189–205, 219–225, 242–270, 290, 311–313, 340–531, 554, 569–571, 663, 736–744, 759–765, 776–791, 796–799, 804–808, 816–827, 843–897, 917–971, 1009–1044
   run_pycpt.py1261260%11–362
   util.py591280%89, 103, 131, 159–172, 179–184
TOTAL157498837% 

Tests Skipped Failures Errors Time
101 0 💤 0 ❌ 0 🔥 13.239s ⏱️

nhsavage and others added 10 commits August 3, 2026 15:39
co-pilot suggested change

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

@Eleanor-Dean Eleanor-Dean left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approved with notes discussed:
--> need to resolve services/centres list so test runs can be easy
--> need to remove some of the terminal output (to verbose)
--> need to set up some regard to coping with the warnings so we dont just forget about them
--> need to do the windows test run
--> need to edit documentations
--> Open above as issues

@nhsavage
nhsavage merged commit e73ef70 into main Aug 5, 2026
4 checks passed
@nhsavage
nhsavage deleted the single_python_script2 branch August 5, 2026 15:14
@nhsavage

nhsavage commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Approved with notes discussed: --> need to resolve services/centres list so test runs can be easy --> need to remove some of the terminal output (to verbose) --> need to set up some regard to coping with the warnings so we dont just forget about them --> need to do the windows test run --> need to edit documentations --> Open above as issues

simple ones covered by #188 , services/centres list clean up already has #183

only missing issue now

  • need to set up some regard to coping with the warnings so we dont just forget about them

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