Skip to content

fix(csv): preserve leading-zero ID columns on read; schema-stable atomic stream writer - #393

Merged
kevincostner17 merged 1 commit into
mainfrom
fix/csv-leading-zeros-and-stream-writer
Sep 15, 2026
Merged

kevincostner17 merged 1 commit into
mainfrom
fix/csv-leading-zeros-and-stream-writer

Conversation

@kevincostner17

Copy link
Copy Markdown
Contributor

Builds on #351 (merged); rebased onto main.

Summary

Leading zeros in CSV input (#228). freshdata clean, freshdata stream and fd.clean_csv read CSV with pandas type inference, so 02134 became 2134 before preserve_leading_zeros could act. A new internal helper, freshdata._csv_io.leading_zero_dtypes, reads a bounded sample (10,000 rows; the first chunk when streaming) as text and returns {column: str} for numeric-looking columns with zero-padded values, using the dtype step's existing _has_leading_zero_ids detector. All three read paths use it, and streaming passes the same mapping to every chunk. The scan is skipped when read_csv_kwargs sets dtype/converters or preserve_leading_zeros=False. Decimal (0.25), negative, all-zero and scientific-notation columns stay numeric.

Schema-stable, atomic stream output (#248, part a). The first batch now fixes the output layout. Later CSV batches are reindexed to its columns, so a missing flag column is written empty instead of shifting values. Later Parquet tables are cast (safe=True) to its schema. An unexpected new column or a failed cast raises a clear ValueError. Output is written to a sibling <output>.partial and moved onto the output path with os.replace on success, or deleted on failure, so a failed run leaves neither a truncated file nor a .partial behind.

Behaviour changes:

  • Zero-padded numeric CSV columns now load as text (non-padded numeric columns are unchanged); a zero-padded column with blanks keeps unpadded values as text (123, not 123.0).
  • freshdata stream exits 1 without writing output when a later batch adds columns or cannot be cast to the first Parquet schema.
  • Padding that first appears after the scanned sample is not detected.

Tests

  • tests/test_csv_leading_zeros.py: CLI clean, multi-chunk CLI stream (and that every chunk gets the same dtype mapping), and fd.clean_csv keep "02134"/"007"; unpadded numeric columns stay numeric; explicit dtype/converters and preserve_leading_zeros=False skip the scan; read_csv_kwargs (e.g. sep) are honoured by the scan.
  • tests/test_stream_writer_schema.py (pyarrow via importorskip): dtype-flipping batches give a well-formed CSV and a readable Parquet with the first schema; a new column (CSV and Parquet) and an uncastable batch raise and leave no output and no .partial (an existing output is untouched); the freshdata stream output breaks when chunk dtypes change: misaligned CSV, partial Parquet #248 CLI reproductions now succeed with no .partial left.

Verification

  • ruff check .: passed
  • mypy src/freshdata: no issues
  • pytest -m "not online and not large" on the rebased branch: Python 3.12 / pandas 2.3.3: 5146 passed, 13 skipped; Python 3.9 / pandas 1.5.3: 5142 passed, 17 skipped

Closes #228
Refs #248 (part a)

…mic stream writer

Leading zeros (#228): `freshdata clean`, `freshdata stream` and
`fd.clean_csv` read CSV input with pandas type inference, so "02134" was
already the integer 2134 before the pipeline's preserve_leading_zeros
handling could see it. A new internal helper, `_csv_io.leading_zero_dtypes`,
reads a bounded sample (10,000 rows; the first chunk for streaming) as
text and returns {column: str} for columns whose values all parse as
numbers and include zero-padded values, using the dtype step's existing
`_has_leading_zero_ids` detector. All three read paths use it; streaming
passes the same mapping to every chunk. The scan is skipped when the
caller sets `read_csv_kwargs` dtype/converters or
preserve_leading_zeros=False, and falls back to plain inference if the
sample cannot be read, so the real read reports its own error.

Stream writer (#248, part a): `_BatchWriter` assumed every batch matched
the first one. CSV batches were appended by position, so a batch without
an anomaly-flag column shifted the other flags under the wrong header.
Parquet raised mid-stream on an int64 -> double change and left a
truncated file. The first batch now fixes the layout: later CSV batches
are reindexed to its columns, later Parquet tables are cast (safe=True)
to its schema, and a new column or a failed cast raises ValueError.
Output is written to a sibling `<output>.partial`, moved onto the output
path with os.replace on success and removed on failure. That leaves an
existing output untouched and no partial file behind. `stream` and
`stream-kafka` both use this commit/abort flow.

Closes #228
Refs #248 (part a)
@coderabbitai

coderabbitai Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 255fde4c-65bf-4fcd-a00c-8d947f287e7f


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

FreshData benchmark report — performance

  • freshdata: ?
  • python: ?
  • platform: ?
fixture n_rows n_cols p50 s p95 s peak MB repair % false-repair % preserve % trust monotonic export %

Authored-code reduction (Metric 6)

@kevincostner17
kevincostner17 merged commit 7b9c380 into main Sep 15, 2026
21 checks passed
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.

freshdata clean, freshdata stream and fd.clean_csv drop leading zeros from ZIP/ID columns

1 participant