Sanity-check all script inputs up front - #41
Open
ggmarshall wants to merge 5 commits into
Open
Conversation
New helpers (utils, all exported): - check_input_files(files, argname): named FileNotFoundError aggregating every missing path (truncated after five). - expand_filelist(..., check_exists=True): expanded filelist members are now existence-checked by default — a missing listed file previously surfaced deep inside lh5.read/load_data, often after other inputs were loaded and DSP work had begun. - prepare_output_paths(*paths): create output parent dirs before the expensive computation instead of at write time. - parse_json_arg(value, argname): JSON CLI args fail naming the argument. - get_rule_config: named error for a nonexistent config directory (dbetto's TextDB raises "input path is not a valid directory" without the path). Script changes: - tier/dsp + tier/hit (both entry points): resolve the rule config via get_rule_config; parse --table-map/--alias-table via parse_json_arg right after build_log (a malformed --alias-table previously failed AFTER the entire tier build); check --input; hoist output dir prep; named error when all --pars-file entries are suffix-filtered away. - dplms/eopt/nopt: build_log now runs before the first config read so failures get ERROR-token logging; inputs checked and output dirs prepared before the optimisation work. - evtsel: wrong --channel on --raw-cal-curve now names the channel and argument (get_channel_config). - svm: --svm-file existence checked (a bad path was silently embedded into the output pars). - svm_build: --train-hyperpars required when --train-data is given (previously Props.read_from(None)); output dir prepared (was missing entirely). - aoe: --detector required when --override-files is given (the override was silently dropped when omitted). - ecal: named errors for --ctc-dict entries all filtered away and for a missing ctc_params key; fixed the dead mkdir-after-open on --results-path; output dirs prepared up front (--save-path had no mkdir at all). - qc: build_log return captured; output dirs prepared up front. - filedb: logging configured before the first file read; --output parent prepared up front (was missing). Tests: check_input_files, prepare_output_paths, parse_json_arg, expand_filelist member checking, get_rule_config missing-dir. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
TextDB.on(category=...) was adopted in #40 but the dependency pin still allowed dbetto 1.3.x, which only accepts the deprecated system argument. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR centralizes “fail fast” input validation for console scripts by adding shared helpers in legenddataflowscripts.utils and adopting them across tier-building and parameter-calibration entry points, so invalid inputs are rejected early with named, actionable errors.
Changes:
- Added input/output/JSON parsing helpers (
check_input_files,prepare_output_paths,parse_json_arg) and strengthenedexpand_filelistby default member existence checking. - Updated multiple scripts to validate required inputs and create output directories immediately after logging setup / config resolution.
- Bumped the
dbettodependency pin to>=1.4to match usage ofTextDB.on(category=...)and avoid test failures on older versions.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_utils_helpers.py | Adds/updates unit tests for new helper behaviors and the new default existence-check semantics. |
| src/legenddataflowscripts/workflow/filedb.py | Prepares output directory before reading config / scanning, after logging is configured. |
| src/legenddataflowscripts/utils/files.py | Introduces check_input_files, prepare_output_paths, parse_json_arg; updates expand_filelist to optionally check existence (default on). |
| src/legenddataflowscripts/utils/cfgtools.py | Adds a named FileNotFoundError when rule config directory does not exist. |
| src/legenddataflowscripts/utils/init.py | Re-exports new helper functions from utils.files. |
| src/legenddataflowscripts/tier/hit.py | Uses get_rule_config, parses JSON args early, validates inputs, and prepares output directories up front. |
| src/legenddataflowscripts/tier/dsp.py | Uses get_rule_config, parses JSON args early, validates inputs, prepares outputs, and adds a named error for filtered-out --pars-file. |
| src/legenddataflowscripts/par/geds/hit/qc.py | Initializes logging into log and prepares output paths early. |
| src/legenddataflowscripts/par/geds/hit/lq.py | Prepares output paths up front (plot/pars/results) rather than immediately before writing. |
| src/legenddataflowscripts/par/geds/hit/ecal.py | Prepares output paths early; adds named errors for filtered --ctc-dict and missing required config keys. |
| src/legenddataflowscripts/par/geds/hit/aoe.py | Prepares output paths early and validates --detector when --override-files is provided. |
| src/legenddataflowscripts/par/geds/dsp/svm.py | Adds existence-check for --svm-file to avoid embedding a bad path. |
| src/legenddataflowscripts/par/geds/dsp/svm_build.py | Prepares output path early; adds validation for training inputs before attempting to train. |
| src/legenddataflowscripts/par/geds/dsp/pz.py | Prepares output paths up front and removes late mkdirs. |
| src/legenddataflowscripts/par/geds/dsp/nopt.py | Reorders logging/config read and introduces early checks/output path preparation. |
| src/legenddataflowscripts/par/geds/dsp/evtsel.py | Uses get_channel_config(..., name="--raw-cal-curve") so bad channels are named in errors. |
| src/legenddataflowscripts/par/geds/dsp/eopt.py | Reorders logging/config read and introduces early checks/output path preparation. |
| src/legenddataflowscripts/par/geds/dsp/dplms.py | Reorders logging/config read and introduces early checks/output path preparation. |
| pyproject.toml | Updates dependency pin from dbetto>=1.3.6 to dbetto>=1.4. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Calibration parameters load from the pars YAML as python (64-bit) floats, and Table.eval/numexpr promotes float32-array x float64-scalar to float64 - making every derived hit column (calibrated energies, PSD and QC classifiers, 83% of the pht tier's raw bytes, compressing at only 0.84-0.95) float64 even when the DSP inputs are float32. YAML cannot express float32, so the cast happens in build-tier-hit after loading the parameter dictionary, in both the multi-channel and single-channel entry points (covers the hit and pht tiers). Ops whose expressions reference genuine float64 columns (timestamp) still promote to float64, which is correct. Validated on production p16 pht pars over real data: 48/49 float ops become float32, all boolean cuts and NaN patterns identical, every difference <= 5e-5 of the column spread. The lone holdout is AoE_Classifier, whose generated expression embeds a 10**-99 literal - to be fixed in the A/E calibration generator upstream. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 19, 2026
Cast hit-operation float parameters to float32 at load time
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-on to #39: every console script now sanity-checks its inputs up front with named errors, so failures surface at startup instead of deep inside processing or after expensive computation.
New helpers (
legenddataflowscripts.utils)check_input_files(files, argname)— namedFileNotFoundErroraggregating every missing path (truncated after five).expand_filelist(..., check_exists=True)— files listed inside filelists are now existence-checked by default. Previously a missing member failed deep insidelh5.read/load_data, often after other inputs were loaded and DSP work had begun.prepare_output_paths(*paths)— output parent dirs are created right afterbuild_log, so an unwritable output location fails before the minutes-long calibration/optimisation, not after it.parse_json_arg(value, argname)— JSON CLI args (--table-map,--alias-table) fail naming the argument. A malformed--alias-tablepreviously failed after the entire tier build.get_rule_config— named error for a nonexistent config directory (dbetto'sTextDBraisesValueError("input path is not a valid directory")without the path).Script changes
get_rule_config(named dir/rule errors instead of the pathless dbettoValueErrorand bareKeyErrors);--table-map/--alias-tableparsed right afterbuild_log;--inputchecked; named error when all--pars-fileentries are suffix-filtered away.build_lognow runs before the first config read, so those failures get ERROR-token logging;--peak-file/--inplotschecked and output dirs prepared before the optimisation.--channelon--raw-cal-curvenames the channel and argument.--svm-fileexistence checked — a bad path was silently embedded into the output pars.--train-hyperparsrequired when--train-datais given (previouslyProps.read_from(None)); output dir prepared (was missing entirely).--detectorrequired when--override-filesis given — omitting it silently disabled the overrides.--ctc-dictentries all filtered away and missingctc_params; fixed the deadmkdir-after-openon--results-path;--save-pathdir prepared (had no mkdir at all).Also
dbetto>=1.4pin: Fix unit-test failure caused by deprecatedTextDB.on()argument #40 adoptedTextDB.on(category=...), which doesn't exist in dbetto 1.3.x still allowed by the previous>=1.3.6pin — the test suite fails withTypeErroron 1.3.7.Tests
54 passing (5 new:
check_input_files,prepare_output_paths,parse_json_arg, filelist member checking,get_rule_configmissing-dir). Existingexpand_filelisttests updated for the existence-check default. legend-dataflow's suite (100 tests) verified green against this branch via its editable install.Behavior note:
expand_filelist's member check is on by default — Snakemake-driven runs are unaffected (inputs are DAG-guaranteed), but hand-run invocations with nonexistent paths now fail at startup with the argument named.Per
AI_POLICY.md: this contribution was drafted with AI assistance (Claude Code) and reviewed by the submitter.🤖 Generated with Claude Code