Fix TBDArray.write_df index handling; raise on Go-binary failure - #19
Merged
Merged
Conversation
TBDArray.write_df passed the DataFrame straight to tiledb.from_pandas, but from_pandas maps the sparse 'time' dimension from the pandas index — a plain 'time' column made every base-class write fail. It also referenced df_val when validate=False, crashing with NameError. write_df now promotes a 'time' column to the index after schema validation (pandera expects it as a column). earthscope-sfg-workflows currently monkeypatches write_df to do this; with this fix that workaround can be dropped. The tiledb_integration Go-binary wrappers (nova2tile, novb2tile, nov0002tile, tdb2rnx) ran subprocess.run(check=False) and returned the CompletedProcess without checking it, so callers that didn't inspect returncode silently treated failed conversions as successes. They now raise RuntimeError on non-zero exit with stdout/stderr in the message, matching the existing GoBinaryRunner behavior in utils/go_runner.py. Also bump the stale pyproject version (0.1.0) to 0.2.1 ahead of the next tag; the 0.2.0 tag shipped while pyproject still said 0.1.0. Note: the round-trip write path cannot be integration-tested in this repo's env — tiledb-py's dataframe layer requires pandas < 3 and the env resolves pandas 3.x (pandas is unpinned here); tests assert on the frame handed to from_pandas instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
✅ Deploy Preview for earthscope-sfg-tools ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
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>
tiledb-py's dataframe layer (tiledb.from_pandas) rejects pandas >= 3, and pandas was unbounded — the repo's own envs resolved pandas 3.x, leaving every TileDB write path broken locally. Bound to <3 and re-lock (pandas 3.0.3 -> 2.3.3 across envs). With from_pandas working again, add a real TileDB round-trip test for TBDArray.write_df alongside the call-capture tests. tiledb_integration.__all__ listed five symbols that were never defined (TileDBService, TileDBOperationResult, TileDBIntegrationError, TileDBBinaryExecutionError, GoBinaryTileDBBackend — leftovers of an abandoned service-layer refactor), which broke star-imports; drop them, add the missing novb2tile/rinex_qc exports, and trim the module docstring to describe what the module actually provides. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
mikegottlieb84
approved these changes
Aug 6, 2026
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
2 participants
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.
Summary
Upstreams two fixes that
earthscope-sfg-workflowscurrently carries as monkey-patches/workarounds at the top ofsv3_pipeline.py(see EarthScope/earthscope-sfg-workflows#23), plus follow-on hygiene:TBDArray.write_dfpassed the DataFrame straight totiledb.from_pandas, butfrom_pandasmaps the sparsetimedimension from the pandas index — a plaintimecolumn made every base-class write fail. It also referenceddf_valwhenvalidate=False(NameError).write_dfnow promotes atimecolumn to the index after pandera validation. The subclass overrides (TDBAcousticArray,TDBShotDataArray) are unchanged — neither has the unbound-variable bug and shotdata's dimensions don't usetime.nova2tile,novb2tile,nov0002tile,tdb2rnx) ransubprocess.run(check=False)and returned without checking the result, so failed conversions were silently treated as successes by any caller not inspectingreturncode. They now raiseRuntimeErroron non-zero exit (stdout/stderr in the message), matching the existingGoBinaryRunnerpattern inutils/go_runner.py. Success still returns theCompletedProcess— callers that already checkreturncodeare unaffected.pandas<3bound: tiledb-py's dataframe layer (tiledb.from_pandas) rejects pandas >= 3, and pandas was unbounded — this repo's own envs resolved pandas 3.0.3, leaving every TileDB write path broken locally. Bounded and re-locked (pandas → 2.3.3), which also enabled a real TileDB round-trip test forwrite_df.tiledb_integration.__all__cleanup: five listed symbols were never defined anywhere (TileDBService,TileDBOperationResult,TileDBIntegrationError,TileDBBinaryExecutionError,GoBinaryTileDBBackend— leftovers of an abandoned service-layer refactor) and broke star-imports; dropped, with missingnovb2tile/rinex_qcexports added.pyproject.tomlversion (0.1.0) to0.2.1— the0.2.0tag shipped while pyproject still said0.1.0. Suggest tagging0.2.1on the merge commit so consumers can pin it.Test plan
tests/test_tiledb_arrays.py: real TileDB round-trip forwrite_dfwithtimeas a plain column (failed before the fix), call-capture tests for index promotion and thevalidate=Falsepath, and raise/return tests for all four Go-binary wrappers. Verified the fix-sensitive tests fail against the unfixed source.pixi run format-check,pixi run -e tiledb build-go,pixi run -e tiledb test→ 87 passed.