-
-
Notifications
You must be signed in to change notification settings - Fork 211
Fix CSV column header spacing in FlightDataExporter #865
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes CSV export headers to remove leading spaces after commas, making column names more accessible when reading with pandas (addresses Issue #864).
- Removed space after comma in CSV header generation for the general export path
- Added comprehensive test to validate CSV column headers have no leading spaces
Reviewed Changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| rocketpy/simulation/flight_data_exporter.py | Removed space after comma in header string concatenation for custom variable exports |
| tests/unit/test_flight_data_exporter.py | Added test to verify CSV headers don't contain leading spaces and are pandas-compatible |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #865 +/- ##
========================================
Coverage 80.27% 80.28%
========================================
Files 104 104
Lines 12769 12769
========================================
+ Hits 10250 10251 +1
+ Misses 2519 2518 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: Gui-FernandesBR <[email protected]>
Co-authored-by: Gui-FernandesBR <[email protected]>
Co-authored-by: Gui-FernandesBR <[email protected]>
900de8d to
b4aef19
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 1 out of 2 changed files in this pull request and generated no new comments.
phmbressan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR looks alright to me. I didn't even know tmp_path was a built-in fixture.
|
@copilot can you kindly update the CHANGELOG.md file to include this PR please? |
Co-authored-by: Gui-FernandesBR <[email protected]>
Fix CSV column header spacing issue in FlightDataExporter (Issue #864)
Problem
The
FlightDataExporter.export_data()method generated CSV files with column headers containing spaces after commas (e.g.,# Time (s), Vz (m/s)). This caused issues when reading the CSV with pandas, as column names would have leading spaces (e.g.,' Vz (m/s)'instead of'Vz (m/s)'), making them harder to access.Solution
Changed line 159 in
flight_data_exporter.pyfrom:to:
This removes the space after the comma, making the header format consistent with the "fast path" implementation which already used comma-only formatting.
Changes
rocketpy/simulation/flight_data_exporter.py: Removed space after comma in header string (line 159)tests/unit/test_flight_data_exporter.py: Added test to verify column headers have no leading spacesCHANGELOG.md: Added entry for this bug fix in the Unreleased sectionBefore and After
Before (OLD):
Pandas columns:
['# Time (s)', ' Z (m)', ' Vz (m/s)', ' Altitude AGL (m)']Access requires:
df[' Vz (m/s)']← Leading space required!After (NEW):
Pandas columns:
['# Time (s)', 'Z (m)', 'Vz (m/s)', 'Altitude AGL (m)']Access:
df['Vz (m/s)']← No leading space needed!Testing
Benefits
✅ Improved Usability: Users can access CSV columns with natural names without leading spaces
✅ Consistency: Both fast path and general path now use the same header format
✅ Backward Compatible: Doesn't break existing functionality, just improves output format
✅ Standards Compliant: More consistent with CSV conventions
This addresses Issue #864 point #10 and the follow-up comment about column name spacing.
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.