Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Use quotes to disambiguate empty and missing strings #138

Closed
Drvi opened this issue Oct 18, 2022 · 0 comments
Closed

Use quotes to disambiguate empty and missing strings #138

Drvi opened this issue Oct 18, 2022 · 0 comments

Comments

@Drvi
Copy link
Collaborator

Drvi commented Oct 18, 2022

For quoted strings, I'd expect that two consecutive quotes represent an empty string (a known value), but I get a missing value instead:

julia> Parsers.xparse(String, "\"\",", 1, 3, Parsers.Options(quoted=true, sentinel=missing)).val.missingvalue
true

Having this would be helpful for CSV parsers which want to differentiate between unknown/missing strings and empty strings (like e.g. Postres does):

"a","","c"   # middle field holds an empty string
"a",,"c"     # missing value in the middle field
quinnj added a commit that referenced this issue Oct 20, 2022
@quinnj
Treat empty quoted field as not empty sentinel for Strings
edeb4dd 
Implements #138.

The biggest question I have is whether we should view this as a bugfix
or a breaking change in behavior. I think it's arguable either way.

As a note, this doesn't break anything in current CSV.jl code,
which is maybe a signal that this change in behavior isn't super
common or makes that much of a difference?

cc: @nickrobinson251, @Drvi
@quinnj quinnj mentioned this issue Oct 20, 2022
Merged
quinnj added a commit that referenced this issue Oct 21, 2022
@quinnj @nickrobinson251
Treat empty quoted field as not empty sentinel for Strings (#139)
4a07611 
* Treat empty quoted field as not empty sentinel for Strings

Implements #138.

The biggest question I have is whether we should view this as a bugfix
or a breaking change in behavior. I think it's arguable either way.

As a note, this doesn't break anything in current CSV.jl code,
which is maybe a signal that this change in behavior isn't super
common or makes that much of a difference?

cc: @nickrobinson251, @Drvi

* Update test/runtests.jl

Co-authored-by: Nick Robinson <npr251@gmail.com>

Co-authored-by: Nick Robinson <npr251@gmail.com>
@quinnj quinnj closed this as completed Oct 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@Drvi @quinnj