-
Notifications
You must be signed in to change notification settings - Fork 43
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
Add Makie recipes #528
Add Makie recipes #528
Conversation
wow! this is so great. I will take a good look. Now, a lot my current code in YAXAarrays will be duplicated, but this is definitely cleaner 😄 |
I'm getting an error like
but tbh I haven't been able to pinpoint it to this change since I hadn't updated DD/Rasters/Makie in a little while (due to issues like rafaqz/Rasters.jl#492). Can you please exemplify how to plot DimArrays with non-default (i.e. not X, Y Z) dimension names? Did you end up going with I it turns out this branch is the source of the error I'll make a MWE asap. |
Ok there was a small bug in this, so checkout the branch again (its also been rebased sorry) But this example should work: A = DimArray(rand(10, 10), (:a, :b))
Makie.heatmap(A; dims=(:a => X, :b => Y)) It ended up making more sense to use Pairs than a namedtuple. (this is in the |
I haven't tested my full plots, but a simple example revals: julia> using DimensionalData, GLMakie
julia> A = DimArray(rand(4, 4), (:a, :b))
4×4 DimArray{Float64,2} with dimensions: Dim{:a}, Dim{:b}
0.170095 0.938657 0.362559 0.275481
0.273901 0.689077 0.618861 0.920637
0.863314 0.834279 0.343351 0.196132
0.645745 0.468384 0.66314 0.0274788
julia> heatmap(A) # Plots things appropriately
julia> heatmap(A, dims=(:b => :X))
ERROR: ArgumentError: Some dims were not found in object.
Stacktrace:
[1] _extradimserror()
@ DimensionalData.Dimensions ~/.julia/packages/DimensionalData/QypyH/src/Dimensions/primitives.jl:744
[2] dimnum
@ ~/.julia/packages/DimensionalData/QypyH/src/Dimensions/primitives.jl:204 [inlined]
[3] permutedims
@ ~/.julia/packages/DimensionalData/QypyH/src/array/methods.jl:205 [inlined]
[4] _permute_xyz(A::DimArray{Float64, 2, Tuple{Dim{:a, DimensionalData.Dimensions.LookupArrays.NoLookup{Base.OneTo{Int64}}}, Dim{:b, DimensionalData.Dimensions.LookupArrays.NoLookup{Base.OneTo{Int64}}}}, Tuple{}, Matrix{Float64}, DimensionalData.NoName, DimensionalData.Dimensions.LookupArrays.NoMetadata}, replacements::Tuple{Dim{:b, X{Colon}}})
@ DimensionalDataMakie ~/.julia/packages/DimensionalData/QypyH/ext/DimensionalDataMakie.jl:412
[5] _permute_xyz(A::DimArray{Float64, 2, Tuple{Dim{:a, DimensionalData.Dimensions.LookupArrays.NoLookup{Base.OneTo{Int64}}}, Dim{:b, DimensionalData.Dimensions.LookupArrays.NoLookup{Base.OneTo{Int64}}}}, Tuple{}, Matrix{Float64}, DimensionalData.NoName, DimensionalData.Dimensions.LookupArrays.NoMetadata}, replacements::Tuple{Pair{Symbol, Symbol}})
@ DimensionalDataMakie ~/.julia/packages/DimensionalData/QypyH/ext/DimensionalDataMakie.jl:405
[6] _permute_xyz
@ ~/.julia/packages/DimensionalData/QypyH/ext/DimensionalDataMakie.jl:404 [inlined]
[7] _prepare_for_makie(A::DimArray{Float64, 2, Tuple{Dim{:a, DimensionalData.Dimensions.LookupArrays.NoLookup{Base.OneTo{Int64}}}, Dim{:b, DimensionalData.Dimensions.LookupArrays.NoLookup{Base.OneTo{Int64}}}}, Tuple{}, Matrix{Float64}, DimensionalData.NoName, DimensionalData.Dimensions.LookupArrays.NoMetadata}, replacements::Pair{Symbol, Symbol})
@ DimensionalDataMakie ~/.julia/packages/DimensionalData/QypyH/ext/DimensionalDataMakie.jl:401
[8] _surface2(A::DimArray{Float64, 2, Tuple{Dim{:a, DimensionalData.Dimensions.LookupArrays.NoLookup{Base.OneTo{Int64}}}, Dim{:b, DimensionalData.Dimensions.LookupArrays.NoLookup{Base.OneTo{Int64}}}}, Tuple{}, Matrix{Float64}, DimensionalData.NoName, DimensionalData.Dimensions.LookupArrays.NoMetadata}, attributes::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}}, dims::Pair{Symbol, Symbol})
@ DimensionalDataMakie ~/.julia/packages/DimensionalData/QypyH/ext/DimensionalDataMakie.jl:150
[9] heatmap(A::DimArray{Float64, 2, Tuple{Dim{:a, DimensionalData.Dimensions.LookupArrays.NoLookup{Base.OneTo{Int64}}}, Dim{:b, DimensionalData.Dimensions.LookupArrays.NoLookup{Base.OneTo{Int64}}}}, Tuple{}, Matrix{Float64}, DimensionalData.NoName, DimensionalData.Dimensions.LookupArrays.NoMetadata}; dims::Pair{Symbol, Symbol}, colorbarkw::NamedTuple{(), Tuple{}}, attributes::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
@ DimensionalDataMakie ~/.julia/packages/DimensionalData/QypyH/ext/DimensionalDataMakie.jl:130
[10] top-level scope
@ REPL[23]:1 I'd expect the previous command to work, since you can infer that DimensionalData.jl/ext/DimensionalDataMakie.jl Lines 22 to 26 in 83ca6dd
Instead I have to manually write out both axes for this to work: |
Another comment is that I've thought about it, and it still seems much simpler to me to take advantage of the notation already used by Makie (i.e. the method heatmap(A, x=:b, y=:a) instead of the current heatmap(A, dims=(:b => :X, :a => :Y)) For me the main selling points are that it's shorter to write and it's more intuitive to Makie users since it an interface that's much closer to Makie's. (The same goes for the Plots interface.) It probably would make it easier to find the appropriate methods to since a quick call to Is there a specific reason why a |
ext/DimensionalDataMakie.jl
Outdated
- `dims`: A `Pair` or Tuple of Pair of `Dimension` or `Symbol`. Can be used to | ||
specify dimensions that should be moved to the `X`, `Y` and `Z` dimensions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One downside of using this same docstring for all methods is that, for example, the Z
dimension doesn't exist in heatmaps and will throw an error if the user tries to use it.
No, it must have broken. I've just tested this: julia> heatmap(A; dims=(:b => Y)) Which somehow works. I don't have actual tests for this because Makie will kill our CI. But I might add a proper test suit to run locally to try to get more edge cases.
Bad syntax doesn't need a reason, its the natural state of code 😅 But seriously my main reason not to use keywords like that is your example: heatmap(x, y, values) x and y are already argument names used in Makie documentation and having them as keywords seems a bit weird.
heatmap(A, x=:a) is pretty concise. I 'll switch to |
Ok the bug is fixed and this should work: heatmap(A; x=:a) |
Ok here's a little test suit of the current state of things: using GLMakie, DimensionalData
# 1d
A1 = rand(X('a':'e'); name=:test)
plot(A1)
scatter(A1)
lines(A1)
scatterlines(A1)
stairs(A1)
stem(A1)
barplot(A1)
waterfall(A1)
# 2d
A2 = rand(X(10:10:100), Y(['a', 'b', 'c']))
A2r = rand(Y(10:10:100), X(['a', 'b', 'c']))
plot(A2)
# Categorical wins: it's on x, even though its Y
boxplot(A2)
boxplot(A2r)
violin(A2)
rainclouds(A2)
# Series also puts Categories in the legend no matter where they are
series(A2)
series(A2r)
# x/y can be specified
A2 = DimArray(rand(10, 10), (:a, :b); name=:stuff)
plot(A2)
contourf(A2; x=:a)
heatmap(A2; y=:b)
@test_throws ArgumentError plot(A2; y=:c)
# 3d
A3 = rand(X(7), Z(10), Y(5))
volume(A3)
A3 = DimArray(rand(10, 10, 5), (:a, :b, :c); name=:stuff)
volume(A3)
volumeslices(A3)
# x/y/z can be specified
A2 = DimArray(rand(10, 10, 7), (:a, :b, :c); name=:stuff)
volume(A3; x=:c)
volumeslices(A3; z=:a) @lazarusA would be good to get feedback from you on these too |
all of your previous tests work for me as well. And at the moment, I don't have other use cases that might be useful. |
I would expect also this one to work (which is not currently): dd = DimArray(rand(m,n), (:a,:b))
series(dd) where the color will need to be |
and maybe similarly,
note: I know too many use cases. We should add more in following PRs. Otherwise this could go on forever. |
Ok using GLMakie
# 1d
A1 = rand(X('a':'e'); name=:test)
plot(A1)
scatter(A1)
lines(A1)
scatterlines(A1)
stairs(A1)
stem(A1)
barplot(A1)
waterfall(A1)
# 2d
A2 = rand(X(10:20:100), Y(['a', 'b', 'c']))
A2r = rand(Y(10:20:100), X(['a', 'b', 'c']))
plot(A2)
# Categorical wins: it's on x, even though its Y
boxplot(A2)
boxplot(A2r)
violin(A2)
rainclouds(A2)
surface(A2)
# Series also puts Categories in the legend no matter where they are
series(A2)
series(A2r)
series(A2r; labeldim=Y)
@test_throws ArgumentError plot(A2; y=:c)
# x/y can be specified
A2ab = DimArray(rand(5, 6), (:a, :b); name=:stuff)
plot(A2ab)
contourf(A2ab; x=:a)
heatmap(A2ab; y=:b)
series(A2ab)
boxplot(A2ab)
violin(A2ab)
rainclouds(A2ab)
surface(A2ab)
series(A2ab)
series(A2ab; labeldim=:b)
# 3d
A3 = rand(X(7), Z(10), Y(5))
volume(A3)
volumeslices(A3)
# x/y/z can be specified
A3abc = DimArray(rand(10, 10, 7), (:a, :b, :c); name=:stuff)
volume(A3abc; x=:c)
volumeslices(A3; z=:a) |
Hmm forgot the series colors. Whats the default colormap for |
it's something like :Set1_5 or something.... and no idea about the resampling, I always do it manually. The issue here is that there are too many options. Is a mix of lines and scatters. @rafaqz you should resample the default theme's colormap for this, IMO. |
Ok above 7 series its now resampling the default |
nice, I tested again, it works. I don't have more feedback at the moment. You maybe should merge. And any issues should be addressed on different PRs :D |
ok. Well, maybe some last one. Just when things are under a million points, maybe.
|
You shouldn't need to convert to |
this one is just a convenient function to plot data cubes. Which might be helpful to have. |
Not really, for all cases I get errors like this one: (maybe more a yax issue ?) using GLMakie, DimensionalData
using YAXArrays
a = YAXArray(rand(3,3)) #|> DimArray
heatmap(a)
|
Ahhh YAXArrays has a bad (so it can detect the dimensions) |
this reads like a job for @felixcremer . I don't know where to start looking at. And for tonight, I'm off. |
No worries, tried the meshscatter but its a bit weird converting a dimension to points, I'm also out for the day (So maybe I'll merge without meshscatter, but we should keep adding things to this later as you say) |
Codecov Report
@@ Coverage Diff @@
## main #528 +/- ##
==========================================
- Coverage 89.68% 82.78% -6.91%
==========================================
Files 39 40 +1
Lines 2909 3223 +314
==========================================
+ Hits 2609 2668 +59
- Misses 300 555 +255
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
@rafaqz sorry I've been offline for a while so I didn't have a chance to interact more before it was merged. For the record though, everything I've tested so far today has worked perfectly. Beautiful work. Thanks so much :) |
I guess one question is: will the overlapping functionality need to be removed from Rasters? |
No worries, thanks for trying it out. When its tagged we can delete the methods in Rasters and set new DD lower bounds. |
Adds a Makie.jl extension and support for most plot types with axis labels and ticks.
Seems a clean way to generate Makie plots with very little code:
@lazarusA maybe you have some ideas for all the things I have missed?
@tomchor this also implements your
dims=:a => X
request for 2d and 3d plots.Closes #524, closes #528 and closes #275
Also @JoshuaBillson this may be of interest with RemoteSensingToolbox for your Makie.jl recipes, and @asinghvi17 as you got all this started! I think most of Rasters.jl Makie recipes could move here so everyone can use it?