JuliaDataCubes / YAXArrays.jl

Yet Another XArray-like Julia package
https://juliadatacubes.github.io/YAXArrays.jl/
Other
98 stars 16 forks source link

Handling abstract types? #410

Open ConnectedSystems opened 1 month ago

ConnectedSystems commented 1 month ago

Not sure if I'm creating false memories but I thought in the past I could create YAXArrays with mixed types?

test_x = stack(Vector{Union{Int,String}}[[1, "Test"], [2, "Test2"]])
YAXArray(test_x)
┌ Warning: Can not determine size of element type. Using DiskArrays.fallback_element_size[] = 100 bytes
└ @ DiskArrays C:\Users\tiwanaga\.julia\packages\DiskArrays\bZBJE\src\chunks.jl:305
╭──────────────────────────────────────╮
│ 2×2 YAXArray{Union{Int64, String},2} │
├──────────────────────────────────────┴──────────────────────── dims ┐
  ↓ Dim_1 Sampled{Int64} Base.OneTo(2) ForwardOrdered Regular Points,
  → Dim_2 Sampled{Int64} Base.OneTo(2) ForwardOrdered Regular Points
├─────────────────────────────────────────────────────────── metadata ┤
  Dict{String, Any}()
├────────────────────────────────────────────────────────── file size ┤
Error showing value of type YAXArrays.Cubes.YAXArray{Union{Int64, String}, 2, Matrix{Union{Int64, String}}, Tuple{DimensionalData.Dimensions.Dim{:Dim_1, DimensionalData.Dimensions.Lookups.Sampled{Int64, Base.OneTo{Int64}, DimensionalData.Dimensions.Lookups.ForwardOrdered, DimensionalData.Dimensions.Lookups.Regular{Int64}, DimensionalData.Dimensions.Lookups.Points, DimensionalData.Dimensions.Lookups.NoMetadata}}, DimensionalData.Dimensions.Dim{:Dim_2, DimensionalData.Dimensions.Lookups.Sampled{Int64, Base.OneTo{Int64}, DimensionalData.Dimensions.Lookups.ForwardOrdered, DimensionalData.Dimensions.Lookups.Regular{Int64}, DimensionalData.Dimensions.Lookups.Points, DimensionalData.Dimensions.Lookups.NoMetadata}}}, Dict{String, Any}}:
ERROR: Argument is an abstract type and does not have a definite size.
Stacktrace:
  [1] sizeof(x::Type)
    @ Base .\essentials.jl:631
  [2] cubesize(c::YAXArrays.Cubes.YAXArray{Union{Int64, String}, 2, Matrix{Union{…}}, Tuple{DimensionalData.Dimensions.Dim{…}, DimensionalData.Dimensions.Dim{…}}, Dict{String, Any}})
    @ YAXArrays.Cubes C:\Users\tiwanaga\.julia\packages\YAXArrays\zyFvF\src\Cubes\Cubes.jl:501
  [3] show_after(io::IOContext{Base.TTY}, mime::MIME{Symbol("text/plain")}, c::YAXArrays.Cubes.YAXArray{Union{Int64, String}, 2, Matrix{Union{…}}, Tuple{DimensionalData.Dimensions.Dim{…}, DimensionalData.Dimensions.Dim{…}}, Dict{String, Any}})
    @ YAXArrays.Cubes C:\Users\tiwanaga\.julia\packages\YAXArrays\zyFvF\src\Cubes\Cubes.jl:512
  [4] show(io::IOContext{Base.TTY}, mime::MIME{Symbol("text/plain")}, A::YAXArrays.Cubes.YAXArray{Union{Int64, String}, 2, Matrix{Union{…}}, Tuple{DimensionalData.Dimensions.Dim{…}, DimensionalData.Dimensions.Dim{…}}, Dict{String, Any}})
    @ DimensionalData C:\Users\tiwanaga\.julia\packages\DimensionalData\BoJag\src\array\show.jl:17
  [5] (::REPL.var"#55#56"{REPL.REPLDisplay{REPL.LineEditREPL}, MIME{Symbol("text/plain")}, Base.RefValue{Any}})(io::Any)
felixcremer commented 1 month ago

This seems to come from the show functions and might have been introduced in newer versions. You can try to suppress the printing by adding a semicolon at the end of the line. Then we would at least know, that it is constructing the YAXArray correctly.

ConnectedSystems commented 1 month ago

Hi @felixcremer

Yes, suppressing the output makes the error go away:

julia> YAXArray(test_x);
┌ Warning: Can not determine size of element type. Using DiskArrays.fallback_element_size[] = 100 bytes
└ @ DiskArrays C:\Users\tiwanaga\.julia\packages\DiskArrays\bZBJE\src\chunks.jl:305
Zapiano commented 3 weeks ago

The problem is not only with AbstractTypes.

I have proposed a fix here: https://github.com/JuliaDataCubes/YAXArrays.jl/pull/421

This error is caused by this call to sizeof(T) https://github.com/JuliaDataCubes/YAXArrays.jl/blob/eb75dfc2ed163e0bf31018061ed0534d6bd99404/src/Cubes/Cubes.jl#L502

The error message comes from the output of sizeof(Union{Int,String})

julia> sizeof(Union{Int,String})
ERROR: Argument is an abstract type and does not have a definite size.
Stacktrace:
 [1] sizeof(x::Type)
   @ Base .\essentials.jl:631
 [2] top-level scope
   @ REPL[61]:1

But that also happens when the type is String, but the message is slightly different (meaning it doesn't happen only with AbstracTypes):

julia> sizeof(String)
ERROR: Type String does not have a definite size.
Stacktrace:
 [1] sizeof(x::Type)
   @ Base .\essentials.jl:631
 [2] top-level scope
   @ REPL[60]:1

I understand that we don't want to prevent users to create YAXArrays of Strings and Unions containing Strings, so I suggest we catch this error and don't show the file size in this specific cases. If the problem was only with AbstractTypes we could use isabstracttype instead, but that doesn't fix the String case.