Skip to content

Conversation

Copy link

Copilot AI commented Oct 29, 2025

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.py from:

exported_header += f", {variable_function.__outputs__[0]}"

to:

exported_header += f",{variable_function.__outputs__[0]}"

This removes the space after the comma, making the header format consistent with the "fast path" implementation which already used comma-only formatting.

Changes

  1. rocketpy/simulation/flight_data_exporter.py: Removed space after comma in header string (line 159)
  2. tests/unit/test_flight_data_exporter.py: Added test to verify column headers have no leading spaces
  3. CHANGELOG.md: Added entry for this bug fix in the Unreleased section

Before and After

Before (OLD):

# Time (s), Z (m), Vz (m/s), Altitude AGL (m)

Pandas columns: ['# Time (s)', ' Z (m)', ' Vz (m/s)', ' Altitude AGL (m)']
Access requires: df[' Vz (m/s)'] ← Leading space required!

After (NEW):

# Time (s),Z (m),Vz (m/s),Altitude AGL (m)

Pandas columns: ['# Time (s)', 'Z (m)', 'Vz (m/s)', 'Altitude AGL (m)']
Access: df['Vz (m/s)'] ← No leading space needed!

Testing

  • All 6 existing FlightDataExporter tests pass
  • All 3 flight export deprecation tests pass
  • New test validates column headers have no leading spaces
  • Verified with pandas that columns are accessible without spaces
  • Code passes ruff linter and formatter checks
  • CodeQL security scan passes (0 alerts)
  • Comprehensive verification confirms fix works
  • CHANGELOG.md updated

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

solve issue #864


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI changed the title [WIP] Fix issue 864 Fix CSV column header spacing in FlightDataExporter Oct 29, 2025
Copilot AI requested a review from Gui-FernandesBR October 29, 2025 02:41
Copilot finished work on behalf of Gui-FernandesBR October 29, 2025 02:41
Copy link

Copilot AI left a 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
Copy link

codecov bot commented Oct 29, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.28%. Comparing base (9cf3dd4) to head (cae8554).
⚠️ Report is 2 commits behind head on develop.

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.
📢 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.

@Gui-FernandesBR Gui-FernandesBR marked this pull request as ready for review November 2, 2025 19:11
@Gui-FernandesBR Gui-FernandesBR requested a review from a team as a code owner November 2, 2025 19:11
Copy link

Copilot AI left a 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.

Copy link
Collaborator

@phmbressan phmbressan left a 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.

@github-project-automation github-project-automation bot moved this from Backlog to Next Version in LibDev Roadmap Nov 20, 2025
@Gui-FernandesBR
Copy link
Member

@copilot can you kindly update the CHANGELOG.md file to include this PR please?

Copy link
Author

Copilot AI commented Nov 21, 2025

@copilot can you kindly update the CHANGELOG.md file to include this PR please?

Updated CHANGELOG.md with the bug fix entry in the Unreleased section. Commit: cae8554

Copilot finished work on behalf of Gui-FernandesBR November 21, 2025 12:23
@Gui-FernandesBR Gui-FernandesBR merged commit b451c38 into develop Nov 21, 2025
18 checks passed
@Gui-FernandesBR Gui-FernandesBR deleted the copilot/fix-issue-864 branch November 21, 2025 12:57
@github-project-automation github-project-automation bot moved this from Next Version to Closed in LibDev Roadmap Nov 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Closed

Development

Successfully merging this pull request may close these issues.

3 participants