Skip to content

[MINOR] off-by-one error in CSV header writing - #2557

Merged
Baunsgaard merged 3 commits into
apache:mainfrom
t99-i:MINOR-Bugfix-FrameRDDConverterUtils
Sep 21, 2026
Merged

Baunsgaard merged 3 commits into
apache:mainfrom
t99-i:MINOR-Bugfix-FrameRDDConverterUtils

Conversation

@t99-i

@t99-i t99-i commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

fixed incorrect column name indexing (off-by-one error) in CSV header writing

fixed incorrect column name indexing (off-by-one error) in CSV header writing
@t99-i

t99-i commented Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

@janniklinde

@janniklinde

Copy link
Copy Markdown
Contributor

Thanks for identifying and fixing the issue @t99-i. It would be great if you could include a test case which would fail with the old implementation.

- added corresponding test which should fail with the old implementation
@codecov

codecov Bot commented Aug 4, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.41%. Comparing base (3de7cbe) to head (dee8ae6).
⚠️ Report is 130 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #2557      +/-   ##
============================================
+ Coverage     71.36%   71.41%   +0.04%     
- Complexity    48688    50714    +2026     
============================================
  Files          1570     1642      +72     
  Lines        188757   197164    +8407     
  Branches      37039    38306    +1267     
============================================
+ Hits         134711   140798    +6087     
- Misses        43594    45290    +1696     
- Partials      10452    11076     +624     
Flag Coverage Δ
java 71.41% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@Baunsgaard
Baunsgaard merged commit 9c3c8a1 into apache:main Sep 21, 2026
48 of 49 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in SystemDS PR Queue Sep 21, 2026
@Baunsgaard

Copy link
Copy Markdown
Contributor

Thanks @t99-i for the bug fix, while merging i just fixed the code style formatting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants