Add config and cli option#1303
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1303 +/- ##
==========================================
- Coverage 89.65% 89.51% -0.15%
==========================================
Files 57 58 +1
Lines 8441 8537 +96
Branches 8441 8537 +96
==========================================
+ Hits 7568 7642 +74
- Misses 565 580 +15
- Partials 308 315 +7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds a new program setting and CLI override to control whether model input files are copied into the run output directory, primarily to avoid unnecessary duplication in generated outputs (e.g., regression test data).
Changes:
- Add
copy_input_filestoSettings(defaulttrue) and document it in the settings schema. - Add
--no-copy-input-filesto theruncommand and gate the existingcopy_input_files(...)call on the setting. - Update
tests/regenerate_test_data.shto use the new CLI flag (though the current update appears inconsistent with the stated goal; see comments).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
tests/regenerate_test_data.sh |
Adds an additional example run using --no-copy-input-files (currently doesn’t prevent copying into the primary data/$example outputs). |
src/settings.rs |
Introduces copy_input_files: bool with default true. |
src/cli.rs |
Adds --no-copy-input-files flag and skips copying when disabled. |
schemas/settings.yaml |
Documents the new copy_input_files setting with default true. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| env MUSE2_LOG_LEVEL=error MUSE2_USE_DEFAULT_SETTINGS=1 \ | ||
| cargo -q run example run -o "data/$example" "$example" \ | ||
| --overwrite $@ | ||
| env MUSE2_LOG_LEVEL=error MUSE2_USE_DEFAULT_SETTINGS=1 \ | ||
| cargo -q run example run -o "data/${example}_debug" "$example" \ | ||
| --overwrite --no-copy-input-files $@ |
There was a problem hiding this comment.
Yeah I don't think we need the second run. I'd just add --no-copy-input-files to the first run
| @@ -161,9 +168,10 @@ pub fn handle_run_command(model_path: &Path, opts: &RunOpts) -> Result<()> { | |||
| .to_str() | |||
| .context("Invalid chars in model directory name")?; | |||
|
|
|||
| copy_input_files(&model_path, output_path, model_name) | |||
| .context("Failed to copy input files to output directory.")?; | |||
|
|
|||
| if settings.copy_input_files { | |||
| copy_input_files(&model_path, output_path, model_name) | |||
| .context("Failed to copy input files to output directory.")?; | |||
| } | |||
alexdewar
left a comment
alexdewar
left a comment
There was a problem hiding this comment.
All looks good except for the double running of the model in regenerate_test_data.sh thing that Copilot flagged.
alexdewar
left a comment
alexdewar
left a comment
There was a problem hiding this comment.
Nearly there, but there a couple of small fixes needed.
There is another weird issue where the permissions for regenerate_test_data.sh have changed and it's not executable any more. Not sure if it still works in Windows (Windows does permissions differently to Unix), but on Linux I get this error:
❯ just regenerate_test_data
sh: line 1: tests/regenerate_test_data.sh: Permission denied
error: recipe `regenerate_test_data` failed on line 23 with exit code 126
You can change it back with:
git add --chmod=+x tests/regenerate_test.data.sh
git commit -m "Fix permissions for `regenerate_test_data.sh`"
alexdewar
left a comment
alexdewar
left a comment
There was a problem hiding this comment.
The permissions were still not fixed, so I've pushed the fix directly.
All good otherwise though. Thanks!
Description
Added a
copy_input_filesoption tosettings.tomland--no-copy-input-filesCLI flag for the run command. When disabled, input files will not be copied to the output folder.Also updated
regenerate_test_data.shto pass--no-copy-input-filesso the test data folder doesn't get cluttered with duplicates of the example model input files.Fixes #1298
Type of change
Key checklist
$ cargo test$ cargo docpresent in the previous release
Further checks