Skip to content

fix: align appended emissions records to existing CSV headers - #1443

Merged
benoit-cty merged 3 commits into
mlco2:masterfrom
nawaaaaaAaar:fix/csv-header-order
Oct 9, 2026
Merged

benoit-cty merged 3 commits into
mlco2:masterfrom
nawaaaaaAaar:fix/csv-header-order

Conversation

@nawaaaaaAaar

@nawaaaaaAaar nawaaaaaAaar commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #1442.

Align append records with the existing CSV's header order before writing a headerless row. has_valid_headers already allows reordered headers, but the append path previously wrote values in dataclass order, corrupting their column associations.

Only the existing header is read, not historical rows. New-file and update-mode behavior is unchanged; all-None columns remain present.

Verification on Python 3.12.13:

  • Real CSV regression fails in append mode before the fix; update-mode control passes.
  • Package test command: 681 passed, two skipped, three integration cases deselected, twelve subtests passed.
  • All applicable autoflake/isort/Black/flake8 hooks passed.

The regression verifies every field in the appended row with csv.DictReader, including identifiers, emissions, energy, and coordinates. Concurrent CSV writers, remote filesystems, visualization and API/dashboard integration were not tested.

AI disclosure: autonomously reproduced, implemented, and tested using Perplexity Computer (GPT-6.1). No human review is claimed.

Combined with #1439 and #1441: 695 passed, two skipped, three integration cases deselected and twelve subtests passed. PR is open and ready for review; hosted verification workflows require maintainer approval. A successful PR-size labeler is not a hosted test pass.

Copilot AI balanced review requested due to automatic review settings October 5, 2026 16:18
@nawaaaaaAaar
nawaaaaaAaar requested a review from a team as a code owner October 5, 2026 16:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@davidberenstein1957 davidberenstein1957 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good fix for #1442: reindexing the appended row to the file's header is safe because has_valid_headers already guarantees the same column set, and the test uses a real CSV. One ordering note for the maintainers: #1419 touches the same file for a problem master has already fixed, so it should be closed before this one merges, otherwise it would conflict.

@codecov

codecov Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.07%. Comparing base (7929a4f) to head (4458cbf).
⚠️ Report is 4 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1443   +/-   ##
=======================================
  Coverage   92.07%   92.07%           
=======================================
  Files          49       49           
  Lines        5199     5202    +3     
=======================================
+ Hits         4787     4790    +3     
  Misses        412      412           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Reuse the header row read for has_valid_headers when reordering the
appended record, and document that reindex() relies on that check.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added size/M and removed size/S labels Oct 9, 2026
@benoit-cty

Copy link
Copy Markdown
Contributor

Thanks,

The test is good: it compares every field of the appended row through csv.DictReader and uses update mode as a control.

I've done small changes: The CSV header is now read once and used both for the check and for reordering the appended row:

  • New helper _read_headers() returns the file's header row, or None if the file is empty.
  • has_valid_headers(data, headers=None) takes the header row as an optional argument and reads the file itself only when none is passed.
  • out() reads the header once, passes it to has_valid_headers, and reuses it for reindex(). That removes the second file open the PR had added.

@benoit-cty
benoit-cty merged commit 8a8776a into mlco2:master Oct 9, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CSV append corrupts column associations when valid headers are reordered

4 participants