rafaqz / DimensionalData.jl

Named dimensions and indexing for julia arrays and other data
https://rafaqz.github.io/DimensionalData.jl/stable/
MIT License
262 stars 38 forks source link

Towards a nicer Makie recipe #720

Open asinghvi17 opened 4 weeks ago

asinghvi17 commented 4 weeks ago

It occurs to me that dimensional axes are dimensional, and we can use Makie's new unit integration to deal with them.

For example, instead of defining conversions in convert_arguments, we might also define expand_dimensions for dimensional arrays, so that Makie can pick up on the dimension of the unit. Defining a custom dim conversion might also allow axis modifications like default labels, without the need to override the basic plot! function.

This isn't particularly urgent, but would be (a) nice to have and (b) pave the way for more such integration in the Makie ecosystem.

asinghvi17 commented 3 weeks ago

I just ran into an issue trying to plot a Raster using GeoMakie :D

Consider the following:

using RasterDataSources, Rasters, ArchGDAL, CairoMakie, GeoMakie
ras = Raster(EarthEnv{HabitatHeterogeneity}, :homogeneity) # habitat homogeneity to neighbouring pixel
surface(ras[:homogeneity]; axis = (; type = GeoAxis))

This causes an error:

ERROR: MethodError: no method matching setindex!(::Nothing, ::Tuple{String, String}, ::Symbol, ::Symbol)
Stacktrace:
 [1] surface(A::Raster{…}; x::Nothing, y::Nothing, colorbarkw::@NamedTuple{}, attributes::@Kwargs{…})
   @ DimensionalDataMakie ~/.julia/dev/DimensionalData/ext/DimensionalDataMakie.jl:143
 [2] top-level scope
   @ ~/.julia/dev/GeoMakie/examples/rasters.jl:13
Some type information was truncated. Use `show(err)` to see complete types.

because the plotting code assumes that surface is always plotted in a LScene, and more importantly it doesn't query what kind of axis has been created. This can be solved independently, but it's something which I feel could be solved more cleanly with this approach.

rafaqz commented 3 weeks ago

This all sounds good to me. But there are potential problems like: we add a Colorbar to some plots, set axis names manually, and add our own attributes to Attributes.

I think we will need an example PR to show if this is viable or not, I suspect currently its not.

Edit: but we should do it anyway, and maybe just keep our own methods until the other things can be moved somewhere else too.

asinghvi17 commented 1 week ago

FYI,

function Makie.expand_dimensions(::Makie.ImageLike, raster::AbstractRaster)
    return Makie.convert_arguments(Makie.ImageLike(), raster)
end

just works. I ran into this issue with GeoMakie's meshimage recently while trying to plot a Raster through it.

We can also avoid evaling things and go through create_plot once https://github.com/MakieOrg/Makie.jl/pull/3974 lands. That gets past the axis issue, but not the colorbar.

rafaqz commented 1 week ago

Are you saying we just need to add that method?

I don't totally follow like what does "just works" mean exactly here

asinghvi17 commented 1 week ago

Yeah we just need to add expand dimensions for each trait to make DimArray handling generic across traits!

asinghvi17 commented 1 week ago

https://github.com/rafaqz/DimensionalData.jl/blob/58dedd6f6a6381f9882bbb59981972be72d73763/ext/DimensionalDataMakie.jl#L353-L357 seems like piracy if I understand correctly?

rafaqz commented 1 week ago

Ah should probably be AbstractDimMatrix?

Maybe a typo

asinghvi17 commented 1 week ago

I'll fix in the PR and see if anything breaks :D

rafaqz commented 1 week ago

Ok beat me to it

asinghvi17 commented 1 week ago

Unfortunately #747 doesn't seem to solve the issue of dimensional axes. I can't actually see where the stack overflow is happening though.