Harden Parquet IO against mixed-type parameter_value columns#118
Merged
Conversation
WQP non-detects arrive as ResultMeasureValue='ND'. In _apply_unit_conversion
numeric results convert to float while float('ND') raises and the raw string is
kept ("Keeping ... for time series data"). The result is a parameter_value
column mixing floats and qualitative strings, which crashed the Parquet handoff:
pyarrow.lib.ArrowInvalid: Could not convert 'ND' with type str: tried to
convert to double ... for column parameter_value
pyarrow infers one logical type per column and can't hold both. Rather than drop
the qualitative markers (they are intentional time-series data), stringify any
object column that mixes numeric values with non-numeric strings before writing.
- backend/persisters/geodataframe.py: new _frame_to_parquet_bytes helper scans
columns and casts mixed numeric/str columns to str, preserving nulls. Both
dicts_to_parquet_bytes (records/sites) and timeseries_to_parquet_bytes
(observations) route through it, since the records path can carry the same mix.
Qualitative markers ('ND', '<0.5') now survive the round-trip; all-numeric
batches keep their float64 dtype. Every downstream consumer already
float()-coerces and skips non-numeric values (backend.trend_stats.daily_series),
so string storage in ND-bearing batches is safe. Note: parameter_value dtype now
varies by batch (float64 when clean, str when any qualitative value present).
Verified against pyarrow: the mixed column round-trips with markers intact and
all-numeric batches stay float64; local persister suite green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Your pull request is automatically being deployed to Dagster Cloud.
|
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
1 participant
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.
Problem
Source steps crash in the Dagster Parquet handoff when an analyte's
parameter_valuecolumn mixes numeric and qualitative values:WQP non-detects arrive as
ResultMeasureValue='ND'; NMED DWB emits below-reporting-limit markers like'< MRL 0.1 MG/L'. In_apply_unit_conversionnumeric results convert tofloatwhilefloat('ND')raises and the raw string is deliberately kept ("Keeping ... for time series data"). pyarrow infers one logical type per column and can't hold both — crashing whichever way it guessed (double → ArrowInvalid, string → ArrowTypeError).Observed sources/analytes:
wqp(arsenic, carbonate),nmed_dwb(calcium, chloride). Any analyte with a non-detect / below-MRL result hits it.Fix
backend/persisters/geodataframe.py: new_frame_to_parquet_byteshelper scans each column and, if it mixes numeric values with non-numeric strings, casts the whole column tostrbefore writing (nulls preserved). Bothdicts_to_parquet_bytes(records/sites) andtimeseries_to_parquet_bytes(observations) route through it. Order-independent — covers both thedouble-inferred andstring-inferred crash variants.Qualitative markers are intentional time-series data, so they are kept rather than dropped. Every downstream consumer already
float()-coerces and skips non-numeric values (backend.trend_stats.daily_series), so string storage is safe.Note:
parameter_valuedtype now varies by batch —float64when clean,strwhen any qualitative value is present. Any future reader must not assume a fixed dtype.Verification
Against real pyarrow: both crash variants (
'ND'/'< MRL ...'with floats, in either value ordering) round-trip with markers intact; all-numeric batches keepfloat64. Local persister suite green.🤖 Generated with Claude Code