Skip to content

Fix DataFrame truthiness crash in _validate_kinfile; surface pdp3 failures - #34

Merged
frigusgulo merged 2 commits into
mainfrom
fix/validate-kinfile-truthiness
Jul 31, 2026
Merged

Fix DataFrame truthiness crash in _validate_kinfile; surface pdp3 failures#34
frigusgulo merged 2 commits into
mainfrom
fix/validate-kinfile-truthiness

Conversation

@frigusgulo

@frigusgulo frigusgulo commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • PrideProcessor._validate_kinfile used if kin_df and not kin_df.empty, which raises ValueError: The truth value of a DataFrame is ambiguous for any existing kin file that parses into a DataFrame. The cached-result check therefore crashed instead of skipping re-processing. Fixed to if kin_df is not None and not kin_df.empty.
  • _run_pdp3 now logs an error when pdp3 exits non-zero (returncode + stderr tail) and when a run produces no kin output, so silent 0-KIN-file runs (cf. pdp3 config_file missing 'AI Ambiguity validation' key → every PPP run silently produces 0 KIN files #28) are visible in logs.
  • process() no longer launches pdp3 when required products are missing — it previously logged Missing required products and ran a doomed pdp3 job anyway. It now raises MissingProductsError (newly exported from pride_ppp). process_batch() stays batch-robust: affected files yield a failed ProcessingResult (returncode == -1, missing specs in stderr) without dispatching pdp3, and other dates process normally.

earthscope-sfg-workflows currently monkeypatches _validate_kinfile at the top of sv3_pipeline.py to work around the truthiness bug — EarthScope/earthscope-sfg-workflows#23 drops that patch in favor of this fix.

Test plan

  • New TestValidateKinfile regression tests: valid kin file → True (raised ValueError before the fix — verified against the unfixed code), missing path → False, override=True → False, unparseable file → False.
  • New TestRunPdp3 tests drive _run_pdp3 through a fake pdp3 script on PATH: outputs are moved/renamed correctly, and non-zero exit + missing kin output are logged.
  • New TestMissingRequiredProducts tests cover the process() raise and the process_batch() failed-result path, with a guard asserting pdp3 is never invoked for an unfulfilled date.
  • uv run pytest packages/pride-ppp/tests/ -q → 49 passed; ruff check / ruff format --check clean.

…lures

_validate_kinfile used `if kin_df and not kin_df.empty`, which raises
`ValueError: The truth value of a DataFrame is ambiguous` whenever an
existing kin file parses into a DataFrame — so the cached-result check
crashed instead of skipping re-processing. earthscope-sfg-workflows
currently monkeypatches the method to work around this; with this fix
that workaround can be dropped.

_run_pdp3 now also logs an error when pdp3 exits non-zero (returncode +
stderr tail) and when a run produces no kin output, so failed runs are
visible in logs instead of only in the ProcessingResult fields.

Regression tests cover the valid-kinfile path (raised before the fix),
missing path, override=True, and unparseable files.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
frigusgulo pushed a commit to EarthScope/earthscope-sfg-workflows that referenced this pull request Jul 31, 2026
sv3_pipeline.py carried three monkey-patches applied at import time:
write_config_file (inject 'ISB model' / 'AI Ambiguity validation' pdp3
keys), PrideProcessor._validate_kinfile (DataFrame truthiness crash),
and TBDArray.write_df (promote 'time' column to index for
tiledb.from_pandas). All three are now fixed at the source:

- pride-ppp: config keys fixed in EarthScope/GNSSommelier#33 (merged),
  _validate_kinfile fixed in EarthScope/GNSSommelier#34
- earthscope-sfg-tools: write_df fixed in
  EarthScope/earthscope-sfg-tools#19 (which also makes the Go-binary
  wrappers raise on non-zero exit instead of failing silently)

pyproject now pins pride-ppp to an explicit GNSSommelier rev (it was
unpinned, resolving default-branch HEAD at lock time) and moves the
sfg-tools pin from 0.2.0 to the fix commit; pixi.lock re-solved
accordingly. Once the upstream PRs merge, both pins should move to
merged-main revs (sfg-tools 0.2.1 tag) with a single re-lock.

Removing the base-class write_df patch also ends qc_pipeline's silent
dependence on sv3_pipeline being imported first for its
qcKinPositionTDB writes to work.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
process() logged 'Missing required products' and then ran pdp3 anyway
with an incomplete config — a run guaranteed to fail downstream, and
invisibly so for callers that only checked the log. It now raises
MissingProductsError (exported from pride_ppp) unless a valid cached
kin file short-circuits the run first.

process_batch() stays batch-robust: files whose date is missing
required products yield a failed ProcessingResult (returncode -1,
stderr lists the missing specs) without dispatching pdp3, and other
dates in the batch process normally.

New tests cover _run_pdp3 via a fake pdp3 script on PATH (output
moving/renaming, non-zero exit + no-kin-output error logging) and both
missing-products paths, with a guard that pdp3 is never invoked for an
unfulfilled date.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@frigusgulo
frigusgulo merged commit b8db24e into main Jul 31, 2026
4 checks passed
@frigusgulo
frigusgulo deleted the fix/validate-kinfile-truthiness branch July 31, 2026 18:13
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.

1 participant