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.

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

Correct error writing struct #961

Merged
merged 5 commits into from
Jan 15, 2022
Merged

Correct error writing struct #961

merged 5 commits into from
Jan 15, 2022

Conversation

jcunwin
Copy link
Contributor

@jcunwin jcunwin commented Jan 14, 2022

When using CSV.write on a array of structs, I want to override the column names of the struct in the CSV header.

using Dates
using CSV

struct StructType
    adate::Date
    astring::Union{String, Nothing}
    anumber::Union{Real, Nothing}
end

structtable = [StructType(Date("2021-12-01"), "string 1", 123.45), StructType(Date("2021-12-31"), "string 2", 456.78)]

io = IOBuffer()

CSV.write(io, structtable; header=["Date Column", "String Column", "Number Column"])

An exception occurs because write.jl tries to use the specified header value ("Date Column") to retrieve the struct's property value instead of the struct property name (:adate), resulting in an exception. E.g.:

CSV.write: Error During Test at /home/john/Code/investments/CSV.jl/test/write.jl:41
  Got exception outside of a @test
  type StructType has no field Date Column
  Stacktrace:
    [1] getproperty
      @ ./Base.jl:42 [inlined]
    [2] getcolumn(x::StructType, nm::Symbol)
      @ Tables ~/.julia/packages/Tables/M26tI/src/Tables.jl:102
    [3] getcolumn
      @ ~/.julia/packages/Tables/M26tI/src/tofromdatavalues.jl:74 [inlined]
    [4] eachcolumn
      @ ~/.julia/packages/Tables/M26tI/src/utils.jl:86 [inlined]
etc...

I.e. It tries something like:

getproperty(Symbol("Date Column"))

instead of something like:

getproperty(Symbol(:adate))

A fix is possible in write.jl in the write function. I.e. instead of using the header names to derive the schema:

# handle unknown schema case
function write(::Nothing, rows, file, opts;
        append::Bool=false,
        compress::Bool=false,
        header::Union{Bool, Vector}=String[],
        bufsize::Int=2^22
    )

#snip

    names = header isa Bool || isempty(header) ? propertynames(row) : header
    sch = Tables.Schema(names, nothing)    # <- Problem is here - wrongly using header values
    cols = length(names)

#snip

    return file
end

... use the data schema directly:

# handle unknown schema case
function write(::Nothing, rows, file, opts;
        append::Bool=false,
        compress::Bool=false,
        header::Union{Bool, Vector}=String[],
        bufsize::Int=2^22
    )

#snip

    names = header isa Bool || isempty(header) ? propertynames(row) : header
    sch = Tables.Schema(propertynames(row), nothing)   # <- fix is here - use property names
    cols = length(names)

#snip

    return file
end

I have created a fork with a code fix and a test case here:

https://github.com/jcunwin/CSV.jl

src/write.jl Outdated Show resolved Hide resolved
src/write.jl Outdated Show resolved Hide resolved
test/write.jl Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Jan 14, 2022

Codecov Report

Merging #961 (ac6d567) into main (482a187) will decrease coverage by 0.32%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #961      +/-   ##
==========================================
- Coverage   90.28%   89.95%   -0.33%     
==========================================
  Files           9        9              
  Lines        2306     2211      -95     
==========================================
- Hits         2082     1989      -93     
+ Misses        224      222       -2     
Impacted Files Coverage Δ
src/write.jl 84.52% <100.00%> (ø)
src/file.jl 95.19% <0.00%> (-0.50%) ⬇️
src/utils.jl 85.98% <0.00%> (-0.43%) ⬇️
src/chunks.jl 91.30% <0.00%> (ø)
src/context.jl 88.81% <0.00%> (+0.16%) ⬆️
src/rows.jl 83.76% <0.00%> (+0.34%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 482a187...ac6d567. Read the comment docs.

@quinnj quinnj merged commit 654baba into JuliaData:main Jan 15, 2022
@quinnj
Copy link
Member

quinnj commented Jan 15, 2022

Thanks @jcunwin for this!

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.

2 participants