Type piracy on CommonDataModel

[rafaqz]

This line is type piracy on CommonDataModel.jl:

NCDatasets.jl/src/subvariable.jl

Line 94 in 66d1e67

Base.view(v::AbstractVariable,indices::Union{Int,Colon,AbstractVector{Int}}...) = SubVariable(v,indices...)

Which causes test failures here: https://github.com/rafaqz/Rasters.jl/actions/runs/5475178088/job/14825206026?pr=416#step:5:802

@tcarion FYI

Should it instead dispatch on SubVariable ?

[Alexander-Barth]

From you comment:

#221 (comment)

Yes, indeed, it would be better to move SubVariable related view function to CommonDataModel.jl.

[rafaqz]

That means the SubVaroable also needs to move to CommonDataModel...

That may be unnnecessary as DiskArrays already handles views pretty well.

If you dont want to use that, we could instead use a Union of all the netcdf variable types rather than AbstractVariable... I'll change the PR to that as a short term fix for now.

[rafaqz]

Loking at it this is a bit of a design problem that CommonDataModel depends on NCDatasets.SubVariable for handling view on all AbstractVariable.

Do you want to instead use DiskArrays.jl SubDiskArray or SubVariable in CommonDataModel.jl ? Even if we move SubVariable to CommonDataModel.jl it will cause dispatch issues with SubDiskArray and the PRs to add DiskArrays support.

We need to make a pretty high level design choice here: DiskArrays.jl or no DiskArrays.jl

[tcarion]

I actually encountered an issue with the view method after making Variable implement DiskArray, see this commit. So this should be fixed when #205 is merged.

[Alexander-Barth]

#205 is merged and the type piracy does no longer exists as SubVariable has been moved to CommonDataModel.