JuliaData / CSV.jl

Utility library for working with CSV and other delimited files in the Julia programming language
https://csv.juliadata.org/
Other
470 stars 140 forks source link

Support groupmark #1093

Closed LilithHafner closed 1 year ago

LilithHafner commented 1 year ago

Closes #626. These features don't implement themselves, even when seven separate people comment asking for them.

See also https://github.com/JuliaData/Parsers.jl/issues/168. This PR fixes none of that.

This is my first time working with this codebase and its style and I may have missed something. I'd appreciate a careful review.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 75.00% and no project coverage change.

Comparison is base (ae05b87) 90.40% compared to head (0e1d923) 90.40%.

:exclamation: Current head 0e1d923 differs from pull request most recent head fdc5253. Consider uploading reports for the commit fdc5253 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1093 +/- ## ======================================= Coverage 90.40% 90.40% ======================================= Files 9 9 Lines 2293 2294 +1 ======================================= + Hits 2073 2074 +1 Misses 220 220 ``` | [Impacted Files](https://app.codecov.io/gh/JuliaData/CSV.jl/pull/1093?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaData) | Coverage Δ | | |---|---|---| | [src/context.jl](https://app.codecov.io/gh/JuliaData/CSV.jl/pull/1093?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaData#diff-c3JjL2NvbnRleHQuamw=) | `88.99% <60.00%> (ø)` | | | [src/chunks.jl](https://app.codecov.io/gh/JuliaData/CSV.jl/pull/1093?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaData#diff-c3JjL2NodW5rcy5qbA==) | `91.30% <100.00%> (ø)` | | | [src/file.jl](https://app.codecov.io/gh/JuliaData/CSV.jl/pull/1093?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaData#diff-c3JjL2ZpbGUuamw=) | `94.42% <100.00%> (ø)` | | | [src/rows.jl](https://app.codecov.io/gh/JuliaData/CSV.jl/pull/1093?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaData#diff-c3JjL3Jvd3Muamw=) | `84.81% <100.00%> (ø)` | | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/JuliaData/CSV.jl/pull/1093/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaData)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

LilithHafner commented 1 year ago

That's the kind of request that leads to well-documented packages. Thank you.

JackDunnNZ commented 1 year ago

After this PR, I am seeing a compile error in conjunction with Parsers 2.4.2, so I think Parsers compat needs to be bumped to 2.5.

pkg> add CSV@0.10.10 Parsers@2.4

julia> using CSV
[ Info: Precompiling CSV [336ed68f-0bac-5ca0-87d4-7b16caf5d00b]
ERROR: LoadError: MethodError: Cannot `convert` an object of type Missing to an object of type Vector{String}

Closest candidates are:
  convert(::Type{T}, ::LinearAlgebra.Factorization) where T<:AbstractArray
   @ LinearAlgebra ~/.julia/juliaup/julia-1.9.1+0.x64.apple.darwin14/share/julia/stdlib/v1.9/LinearAlgebra/src/factorization.jl:59
  convert(::Type{Array{S, N}}, ::PooledArrays.PooledArray{T, R, N}) where {S, T, R, N}
   @ PooledArrays ~/.julia/packages/PooledArrays/DXlaI/src/PooledArrays.jl:499
  convert(::Type{T}, ::AbstractArray) where T<:Array
   @ Base array.jl:613
  ...

Stacktrace:
  [1] Parsers.Options(refs::Missing, sentinel::UInt8, ignorerepeated::UInt8, ignoreemptylines::UInt8, wh1::UInt8, wh2::UInt8, quoted::UInt8, oq::UInt8, cq::Vector{String}, e::Vector{String}, delim::Nothing, decimal::Bool, trues::Bool, falses::Nothing, dateformat::Bool, cmt::Bool, stripwhitespace::Bool, stripquoted::Bool, groupmark::Nothing)
    @ Parsers ~/.julia/packages/Parsers/aQvnF/src/Parsers.jl:61

FWIW, it seems like the PR exposed a bug where the parsingdebug and stripwhitespace flags had actually been getting passed as the values to stripwhitespace and stripquoted in older versions of Parsers, as the debug flag was only added in Parsers 2.5

LilithHafner commented 1 year ago

That is unfortunate—I imagine the best fix is the stop mixing up those arguments rather than restricting compat. Unfortunately, I do not have the bandwidth to debug that rn. Perhaps @quinnj is willing to look into it?

quinnj commented 1 year ago

Good catch @JackDunnNZ; mind making a PR to bump Parsers compat up to 2.5?