Use header index to replace fixed column order#77
Use header index to replace fixed column order#77coketaste merged 1 commit intocoketaste/refactor-disfrom
Conversation
…performance, metric, and status show in the correct column
There was a problem hiding this comment.
Pull request overview
Updates performance result recording/display so appended perf.csv rows align with the existing file’s header schema (instead of relying on a fixed column order), ensuring key fields like model/performance/metric/status appear in the correct columns.
Changes:
- In Kubernetes result collection, adjust successful run entries for
multiple_resultsfallback so CLI display uses per-row perf data (by settingnodes=[]). - In Kubernetes perf.csv writing, detect and reuse an existing
perf.csvheader to map values by column name when appending. - In CLI utilities, improve formatting/handling of empty values and
nanmodel names when rendering tables.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/madengine/deployment/kubernetes.py |
Reworks perf.csv appending logic to respect existing header order and tweaks result payload for multi-row fallback display. |
src/madengine/cli/utils.py |
Improves table rendering robustness for empty values and nan model fields. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| file_exists = perf_csv_path.exists() | ||
| existing_header = None | ||
| if file_exists: | ||
| # Read existing header so we write in the same column order (fixes MAD-private | ||
| # and other repos that use a different perf.csv schema with more/different columns) | ||
| with open(perf_csv_path, "r", newline="", encoding="utf-8", errors="replace") as rf: | ||
| reader = csv.reader(rf) | ||
| existing_header = next(reader, None) | ||
| headers = existing_header if existing_header else default_headers | ||
| if file_exists and existing_header: | ||
| # Build row by name so model/performance/metric/status go to correct columns |
There was a problem hiding this comment.
If perf.csv exists but is empty (or the first row is blank), perf_csv_path.exists() will be True while existing_header is None/empty, which skips writeheader() and appends a data row without any header line. Consider treating an empty/invalid header as a new file (e.g., check stat().st_size == 0 or validate existing_header) and write default_headers before appending rows.
| with open(perf_csv_path, "a", newline="", encoding="utf-8") as f: | ||
| writer = csv.DictWriter(f, fieldnames=headers, extrasaction="ignore") | ||
| if not file_exists: | ||
| writer.writeheader() | ||
|
|
||
| # Write data row (only fields in headers will be written) | ||
| writer.writerow(perf_data) | ||
| if file_exists and existing_header: | ||
| csv.writer(f).writerow(row_to_write) | ||
| else: | ||
| writer.writerow(row_to_write) |
There was a problem hiding this comment.
This branch builds writer = csv.DictWriter(...) but then writes rows using csv.writer(f).writerow(...). Mixing DictWriter and writer can lead to subtle differences in quoting/dialect and makes the code harder to follow. Consider consistently using DictWriter.writerow(...) (it already respects fieldnames order and fills missing keys with restval) to write both new and existing-header rows.
Appended rows now match the existing header, model, performance, metric, and status show in the correct column