Skip to content

[pkl.csv] Report empty header columns clearly - #243

Open
abhiishek340 wants to merge 3 commits into
apple:mainfrom
abhiishek340:csv-empty-header-error
Open

abhiishek340 wants to merge 3 commits into
apple:mainfrom
abhiishek340:csv-empty-header-error

Conversation

@abhiishek340

Copy link
Copy Markdown

Closes #76.

Empty cells already parse correctly when the input's line breaks match lineBreak:

new csv.Parser { input = "A,B,C\r\n1,,3"; includeHeader = true }.parsed
// List(new { A = "1"; B = null; C = "3" })

The failure in #76 comes from line breaks. lineBreak defaults to "\r\n" (RFC 4180), and the input in the issue is a Pkl multi-line string, which uses "\n". With no "\r\n" in the input, it's parsed as a single row. The empty cell from the second line then lands in the header, and the as List<String>? cast fails with:

Expected value of type `String`, but got `null`.

This PR replaces that with a message that names the problem and, when the input contains "\n" but no lineBreak, how to fix it:

Column 4 of the header row is empty, so it can't be used as a column name. The input contains no "\r\n" line breaks, so it was parsed as a single row. If it uses "\n" line breaks, set `lineBreak = "\n"`.

A header with a genuinely empty column ("A,,C\r\n1,2,3") gets just the first sentence.

I didn't change the default line break or start accepting both, since that changes parsing semantics. Happy to do that instead if you'd prefer.

Changes:

  • csv.pkl: check the header row for empty columns before the cast, plus a _lineBreakHint for the single-row case.
  • tests/csv_test.pkl: new "Parsing with an empty header column" fact covering both messages and the lineBreak = "\n" case.
  • PklProject: bump pkl.csv to 1.1.2. No other package depends on pkl.csv.

Testing: pkl test packages/pkl.csv/tests/csv_test.pkl passes with Pkl 0.32.1 (the version in libs.versions.toml) and with pkl main: 13 tests, 55 assertions. pkl format (0.32.1) reports no changes in the new code. The three lines it does flag in csv.pkl were already flagged on main, so I left them alone.

Parsing with `includeHeader = true` failed with "Expected value of type
`String`, but got `null`" when a header cell was empty.

The common cause is input that uses "\n" line breaks while `lineBreak`
defaults to "\r\n": the whole input is parsed as one row, so an empty
cell in a later line becomes part of the header. Pkl multi-line strings
use "\n", which is how the input in apple#76 was written.

Report which header column is empty, and when the input contains "\n"
but no `lineBreak`, suggest setting `lineBreak = "\n"`.

Closes apple#76

@bioball bioball left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you!

Comment thread packages/pkl.csv/csv.pkl Outdated
Comment thread packages/pkl.csv/csv.pkl Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[csv] Parser failed when a cell doesn't have value

2 participants