fix: drop dtype coercion in CSV update path - #1370
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1370 +/- ##
==========================================
- Coverage 91.70% 91.69% -0.01%
==========================================
Files 49 49
Lines 5157 5152 -5
==========================================
- Hits 4729 4724 -5
Misses 428 428 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Rebased on fix/csv-update-dtype-coercion (#1370), which fixes the CSV dtype coercion properly, so the local workaround in file.py is dropped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Rebased on fix/csv-update-dtype-coercion (#1370), which fixes the CSV dtype coercion properly, so the local workaround in file.py is dropped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Updating a run row coerced each incoming value to the dtype pandas inferred
for the existing column. Columns that are empty in every row are read back
as float64, so `numpy.float64("")` / `numpy.float64(None)` raised and the
update failed.
Rebuild the row via concat instead, which dedupes by run_id without touching
dtypes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2c331f7 to
e056063
Compare
Verdict: ✅ ApproveWhat it does: instead of casting each value to the column's dtype, it now drops the old row and appends the new one. This fixes the crash when a column is empty in every row. What I checked:
Before merge:
Nits (optional):
|
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Made the changes in 6f035dc: merged master, test now uses None for empty coordinates. |
Description
FileOutput.out()inon_csv_write="update"mode previously assigned new values column by column withdf[col].dtype.type(val). The single-existing-row branch now drops the previous row for the run and re-appends the new one viapd.concat, achieving the same dedupe-by-run_idwithout any dtype coercion. Thelen(df_run) > 1warning branch is unchanged, and the "no existing row" case falls out of the sameconcat.Related Issue
Fixes #1367
Motivation and Context
df[col].dtype.type(val)coerced each incoming value to whatever dtype pandas inferred when reading the existing CSV back. A column that is empty in every existing row is read back asfloat64, so this evaluatednumpy.float64("")ornumpy.float64(None)and raised. This is reachable in normal use: anOfflineEmissionsTrackerleaveslongitude/latitudeempty, andgpu_count/gpu_modelare empty on CPU-only machines, so any run writing twice (aflush()thenstop()) crashed on the second write withValueError: could not convert string to float: ''.How Has This Been Tested?
test_file_output_out_update_with_always_empty_columnsfails onmasterwith theValueErrorabove and passes with this change.uv run pytest tests/output_methods/ -q— 48 passed.uv run pytest tests/test_emissions_tracker.py -q— 31 passed.uv run pre-commit run --files codecarbon/output_methods/file.py tests/output_methods/test_file.py— all hooks pass.Screenshots (if appropriate):
N/A
Types of changes
AI Usage Disclosure
Checklist:
Note on row ordering
The updated row now moves to the end of the file instead of staying in place. Rows are timestamped, so this only affects files where runs were interleaved. Also note: #1366 (SLURM) touches this same block incidentally; whichever lands second will need a rebase.