Fixes for Parsers changes

[quinnj]

No description provided.

[nickrobinson251]

still think the EOF changes need some more investigation --- i'm not sure Parsers.jl is behaving consistently

The other 2 changes LGTM

[nickrobinson251]

did you mean to add this in this PR?

[quinnj]

I did not! Thanks for noticing

[nickrobinson251]

🤔 I don't quite follow what's happening with EOF in Parsers.jl now (or before for that matter)

with Parsers.jl v2.4.2

julia> Parsers.codes(Parsers.xparse(String, "a,").code)
"SUCCESS: OK | DELIMITED | EOF "

julia> Parsers.codes(Parsers.xparse(String, "a,\n").code)
"SUCCESS: OK | DELIMITED "

julia> Parsers.codes(Parsers.xparse(String, "a,,"; ignorerepeated=true).code)
"SUCCESS: OK | DELIMITED | EOF "

julia> Parsers.codes(Parsers.xparse(String, "a,,\n"; ignorerepeated=true).code)
"SUCCESS: OK | DELIMITED | NEWLINE | EOF "

and now on Parsers#main:

julia> Parsers.codes(Parsers.xparse(String, "a,").code)
"SUCCESS: OK | DELIMITED "

julia> Parsers.codes(Parsers.xparse(String, "a,\n").code)
"SUCCESS: OK | DELIMITED "

julia> Parsers.codes(Parsers.xparse(String, "a,,"; ignorerepeated=true).code)
"SUCCESS: OK | DELIMITED "

julia> Parsers.codes(Parsers.xparse(String, "a,,\n"; ignorerepeated=true).code)
"SUCCESS: OK | DELIMITED | NEWLINE | EOF "

^ this last case "a,,\n" still looks odd to me

[nickrobinson251]

looks like finddelimiter is inconsistent on whether or not it looks for newlines after finding delimiter(s):
if ignorerepeated === false, we break as soon as we hit a delim; if ignorerepeated === true, we keep going even if we don't match (another) delim and look for newlines before finally we break if we find neither
https://github.com/JuliaData/Parsers.jl/blob/388613c73ab62341702a6af051577384235b21fc/src/components.jl#L272-L293

[quinnj]

Yeah, I'm not exactly sure how, but the 2.4.2 version for Strings was just wrong; so I'm going to make the tests here compatible, but at least moving forward they will be more consistent.

v2.4.2

julia> Parsers.codes(Parsers.xparse(String, "a,").code)
"SUCCESS: OK | DELIMITED | EOF "

julia> Parsers.codes(Parsers.xparse(Int, "1,").code)
"SUCCESS: OK | DELIMITED "

#main

julia> Parsers.codes(Parsers.xparse(String, "a,").code)
"SUCCESS: OK | DELIMITED "

julia> Parsers.codes(Parsers.xparse(Int, "1,").code)
"SUCCESS: OK | DELIMITED "

[nickrobinson251]

yeah, i think that sounds good

it's this apparent inconsistency on main i think is still odd:

julia> Parsers.codes(Parsers.xparse(String, "a,\n").code)  # seems correct
"SUCCESS: OK | DELIMITED "

julia> Parsers.codes(Parsers.xparse(String, "a,,"; ignorerepeated=true).code)  # seems correct
"SUCCESS: OK | DELIMITED "

julia> Parsers.codes(Parsers.xparse(String, "a,,\n"; ignorerepeated=true).code)  # why `NEWLINE | EOF`  here?
"SUCCESS: OK | DELIMITED | NEWLINE | EOF "

[quinnj]

Hmmm yes, that does seem off. Let me look into it.

[nickrobinson251]

i'm guessing this is something to do with assuming ignorerepeated=true is only ever used with delim=' '? e.g. we don't want accidental trailing whitespace to cause an extra empty column or ever row in CSV, or something like that?

But i think this case where delim=',' and ignorerepeated=true shows it's pretty odd that ignorerepeated changes what happens after we stop hitting delim chars

[quinnj]

Yeah, thinking about the delim=' ' case helps make the most sense of this, and I think drives the requirement here. The general idea of ignorerepeated is "match any number of delimiters until we hit another field or the next row".

[nickrobinson251]

i can kinda understand the NEWLINE code being added for "a,,\n", but why does it get EOF (when "a,," doesn't get EOF)?

[quinnj]

Ah, now that does seem like a good inconsistency (er....bad inconsistency?). Yeah, it seems like that's wrong.

[quinnj]

Aight, thanks for catching that Nick; got a fix here: JuliaData/Parsers.jl@883cfdc. CSV.jl tests seemed happy locally.