feat: add comment character support to CSV parser

[Pcmhacker-piro]

Description

Adds comment parameter support to read_csv(), read_csv_chunked(), and scan_csv(), allowing users to skip comment lines (e.g., lines starting with #) during CSV parsing.

Fixes #47

Root Cause

CsvConfig had no option for specifying a comment character. Lines starting with comment markers (e.g., #) were parsed as regular data rows, producing malformed output.

Implementation

C++ Core (cpp/)

• csv_reader.h: Added std::optional<char> comment_char to CsvConfig and is_comment_line() method to CsvParser
• csv_reader.cpp: Skip comment records at every stage — before header reading, during skip_rows, and in both data-reading passes. Works in CsvReader::read(), CsvReader::scan_schema(), CsvChunkReader::open(), and CsvChunkReader::read_one_data_row(). Comment detection checks the first non-whitespace character of each complete record, so # inside quoted fields is never treated as a comment.

Python Bindings (bindings/)

• Exposed comment_char property on _CsvConfig via pybind11

Python API (arnio/io.py)

• Added _validate_comment_char() validation function
• Added comment: str | None = None parameter to read_csv(), read_csv_chunked(), and scan_csv()

Tests (tests/test_csv.py)

18 new tests covering:

• Basic comment line skipping (with and without leading whitespace)
• Comments inside quoted fields preserved as data
• Default behavior unchanged when comment is not specified
• Different comment characters (#, %)
• Integration with nrows, skiprows, has_header=False
• scan_csv and read_csv_chunked comment support
• Edge cases: all-comment files, empty lines, record numbering
• Invalid comment parameter validation

Testing

• All 2355 existing tests pass (0 failures, 47 skipped)
• 18 new comment-specific tests all pass
• No regressions in any module

[vercel]

Someone is attempting to deploy a commit to the xtylishanish-gmailcom's projects Team on Vercel.

A member of the Team first needs to authorize it.

[im-anishraj]

@Pcmhacker-piro thanks for the detailed PR and test coverage. I added the GSSoC classification labels for this PR (gssoc, level:advanced, type:feature, type:testing, area:csv-parser, area:cpp-core, area:python-api).

I reviewed the diff and ran the local formatting checks. This is not merge-ready yet because CI lint is failing and I can reproduce the Python formatting issue locally:

python -m black --check arnio/io.py tests/test_csv.py

tests/test_csv.py needs Black formatting; the long write_text(...) calls in the new comment tests are the immediate cause.

I also found one parser edge case to fix before this can merge: CsvReader::scan_schema() skips comments while looking for a header, but it does not handle EOF/all-comment files safely, and in the has_header=False branch it reads the first record without skipping comment lines first. That can make scan_csv(..., has_header=False, comment="#") infer schema from a comment row instead of the first data row. Please add focused tests for those scan paths and update scan_schema() to skip comment records consistently before choosing the header/first data row.

Please fix formatting, add the missing scan-schema edge tests, and rerun the targeted CSV tests/CI. I’ll re-review once those are updated.

[im-anishraj]

@Pcmhacker-piro thanks for the quick update. The scan-schema edge cases are now covered in the PR diff, but CI still has a real lint/type-check blocker.

GitHub Actions lint failure:

arnio/io.py:631: error: "CsvConfig" has no attribute "comment_char"  [attr-defined]
arnio/io.py:838: error: "CsvConfig" has no attribute "comment_char"  [attr-defined]
arnio/io.py:1079: error: "CsvConfig" has no attribute "comment_char"  [attr-defined]

The C++ binding exposes comment_char, but the Python native stub also needs to be updated so mypy knows _CsvConfig has this attribute. Please add the new attribute to arnio/_arnio_cpp.pyi and rerun the lint workflow.

Local checks I ran on the latest commit:

git diff --check origin/main...HEAD
python -m black --check arnio/io.py tests/test_csv.py
python -m ruff check arnio/io.py tests/test_csv.py

Those Python formatting/static checks passed locally. I did not approve/merge because CI is still failing and the mypy stub issue must be fixed first.

[im-anishraj]

@Pcmhacker-piro update: I see the follow-up stub commit on this branch, and the latest GitHub Actions lint job is now passing. I am waiting for the remaining CI jobs to finish before doing final review/approval. No extra action is needed from you right now unless one of the remaining jobs fails.

[Pcmhacker-piro]

@im-anishraj,

I’ve fixed this issue so now you can assign me another one.

[im-anishraj]

@Pcmhacker-piro I rechecked the latest commit after the stub fix. The previous requested changes are addressed: the stub now includes comment_char, the scan-schema comment edge cases are covered, and the GitHub Actions matrix is green apart from the external Vercel authorization status.

Approved from code review. I still cannot assign you another issue until this assignment is actually completed, which means PR #1940 is merged or the assignment is closed. Please wait for final maintainer-side Vercel/merge handling before requesting another issue.

[im-anishraj]

Hi @Pcmhacker-piro, thanks for the solid work on comment-character support. I rechecked this after the latest CSV parser merge, and the PR is now showing mergeStateStatus: DIRTY, likely because cpp/src/csv_reader.cpp changed on main.

Could you please update/rebase your branch against the latest main and resolve the conflict? Once it is clean again, I can rerun the focused CSV checks and continue the merge review.

[Pcmhacker-piro]

@im-anishraj
please check this pr

[Pcmhacker-piro]

heyy @im-anishraj
i am removing this conflicts now.
then after this you can review it

[im-anishraj]

@Pcmhacker-piro thanks for pushing the conflict-resolution update. I rechecked the current head and this still cannot merge yet because a non-Vercel lint check is failing.

The current CI lint job says Black would reformat tests/test_csv.py near the end of the file, around the stream.read_sizes assertions. Please run Black locally and push the formatted result.

The test matrix is also reporting test_website_api_reference_mentions_public_exports because current main is missing rename_columns_matching from the website API reference. That appears to be maintainer-side API-doc drift rather than your comment-character change. Vercel is also external/auth-limited and not your blocker.

Once formatting is green again, I can re-review the earlier BOM/comment and public docstring blockers on this updated head.

[Pcmhacker-piro]

heyyy @im-anishraj
now i finally fix this issue #40 so please check it
if you found any bug the inform me

[im-anishraj]

@Pcmhacker-piro I rechecked the latest head after your conflict/lint cleanup. The non-Vercel checks are green now, so the Black formatting blocker from my last review is cleared. Vercel is still external/auth-limited and not your blocker.

I am leaving this in the maintainer review queue to recheck the earlier functionality/docstring concerns on the updated head.

[Pcmhacker-piro]

heyy @im-anishraj

Thanks for the recheck and update. I’ll wait for the maintainer review and address any further feedback if needed.

[im-anishraj]

@Pcmhacker-piro I rechecked the current PR state while you are waiting for maintainer review. GitHub now reports mergeStateStatus: DIRTY, so I cannot continue final review or merge handling yet even though the non-Vercel checks on the current head are green.

Please update/rebase the branch against the latest main and resolve the merge conflicts. Once the branch is clean again, ask for another review here and I can re-check the remaining functionality/docstring concerns. The Vercel authorization failure is still external and not your blocker.

[Pcmhacker-piro]

Hi @im-anishraj, I’ve resolved the merge conflicts and updated the branch. Kindly re-review the PR. Thanks 🙌

[im-anishraj]

@Pcmhacker-piro thanks for resolving the merge conflicts and updating the branch. I rechecked the current PR state: GitHub no longer reports mergeStateStatus: DIRTY (it is now UNSTABLE), and the non-Vercel checks on the latest head are green.

I am moving this back from status:blocked to the maintainer review queue. I am not approving from this notification pass because the earlier functionality/docstring review items still need a maintainer re-review on the updated head. The Vercel authorization failure is still external noise, not your code/test blocker.

[im-anishraj]

@Pcmhacker-piro, thank you for repeatedly keeping this branch current. I completed the maintainer re-review and the two earlier functional/documentation blockers still remain on the latest diff, so I am moving this to status:blocked.

1. Comment detection runs before UTF-8 BOM stripping. A first line such as BOM + # comment is therefore treated as a header/data record instead of a comment. Please make is_comment_line() BOM-aware or strip the BOM before comment detection in the initial header/data paths. Add focused tests for read_csv(), scan_csv(), and preferably read_csv_chunked().
2. Add the new comment parameter to the parameter documentation for read_csv(), read_csv_chunked(), and scan_csv(), including that only a record whose first non-whitespace character matches is skipped and quoted # values remain data.

The non-Vercel CI on the current head is green, but these correctness and public-documentation items must be fixed before approval. Vercel is not a blocker.