Skip to content

Rlg compatbump #55

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.

Sign up for GitHub

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

Closed
lwhitefox wants to merge 2 commits into from
Closed

Rlg compatbump #55

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7b9d0d3
Update README.md
lwhitefox Dec 31, 2024
733433f
Bump compat versions to be current; update Pyramid(path) as needed
lwhitefox Dec 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Project.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ Aqua = "0.8"
ArchGDAL = "0.10.4"
CairoMakie = "0.12"
Colors = "0.12"
DimensionalData = "0.27.3"
DimensionalData = "0.29.5"
DiskArrayEngine = "0.1.2"
DiskArrayTools = "0.1.10"
DiskArrays = "0.4.2"
Expand All @@ -49,8 +49,8 @@ Proj = "1.7.1"
Statistics = "1.10"
Test = "1.10"
TestItemRunner = "0.2.3"
YAXArrayBase = "0.6.1"
YAXArrays = "0.5.6"
YAXArrayBase = "0.7.5"
YAXArrays = "0.6.0"
Zarr = "0.9.3"
julia = "1.9"

Expand Down
3 changes: 2 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ savecube(elev, zarrpath)

To reproduce the zooming example shown at the top of the README you can get the Above Ground Biomass here or try to load it from the cloud, but this might have some serious lag.
```julia
using YAXArrays, PyramidScheme
using YAXArrays
using PyramidScheme:PyramidScheme as PS
using GLMakie
p2020 = PS.Pyramid("https://s3.bgc-jena.mpg.de:9000/pyramids/ESACCI-BIOMASS-L4-AGB-MERGED-100m-2020-fv4.0.zarr")
replacenan(nanval) = data -> <=(nanval)(data) ? NaN32 : Float32(data)
Expand Down
9 changes: 5 additions & 4 deletions src/PyramidScheme.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ end
Pyramid(path::AbstractString) = Pyramid(path, YAB.backendfrompath(path)(path))
function Pyramid(path::AbstractString, backend)
#This should rather be solved via dispatch, but this is not working because of Requires in YAXArrayBase.
if backend isa YAB.ZarrDataset
if (findfirst(x->x==:zarr,[keys(YAB.backendlist)...])!=nothing) && (typeof(backend) == YAB.backendfrompath("test.zarr"))
Copy link
Member

@felixcremer felixcremer Jan 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed?
Shouldn't the backendtype from "test.zarr" always be a ZarrDataset?

Copy link
Contributor Author

@lwhitefox lwhitefox Jan 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree it is not ideal - the comment (not from me!) already shows there's some complications from YAXArrayBase. I don't know YAZArrayBase well, but the backend type returned from the backendfrompath is not easy to work with, which is why I set it up to avoid hard coding a type. But this may be just the limits of my knowledge of how the relevant pkgs set up types (and how to access them)

For example:

julia> using Zarr, YAXArrayBase

julia> tmp = YAXArrayBase.backendfrompath("test.zarr")
ZarrExt.ZarrDataset

julia> tmp <: ZarrDataset
ERROR: UndefVarError: `ZarrDataset` not defined
Stacktrace:
 [1] top-level scope
   @ REPL[23]:1

julia> tmp <: ZarrExt.ZarrDataset
ERROR: UndefVarError: `ZarrExt` not defined
Stacktrace:
 [1] top-level scope
   @ REPL[24]:1

Copy link
Member

@felixcremer felixcremer Jan 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the problem is that the ZarrDataset type is defined in a package extension. I will have a look tomorrow how to improve it.

Copy link
Contributor Author

@lwhitefox lwhitefox Jan 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pkg extensions are cool - but here it makes it hard to test the types, which creates challenges - if an extension is not loaded the code will error since the running code will not know about the :zarr backend, for example. That's why I put the first half of the test logic in (if (findfirst(x->x==:zarr,[keys(YAB.backendlist)...])!=nothing)) to make sure the :zarr backend is active before testing for a zarr file.

I'd be happy to have a simpler way to deal with it though!

_pyramid_zarr(path)
elseif backend isa YAB.GDALDataset
elseif (findfirst(x->x==:gdal,[keys(YAB.backendlist)...])!=nothing) && (typeof(backend) <: YAB.backendfrompath("test.tif"))
_pyramid_gdal(path)
else
throw(ArgumentError("""
Expand Down Expand Up @@ -314,7 +314,8 @@ The data is aggregated with the specified `resampling_method`.
Keyword arguments are forwarded to the `fill_pyramids` function.
"""
function buildpyramids(path::AbstractString; resampling_method=mean, recursive=true, runner=LocalRunner, verbose=false)
if YAB.backendfrompath(path) != YAB.ZarrDataset
# if YAB.backendfrompath(path) != YAB.ZarrDataset
if !(YAB.backendfrompath(path) <: YAB.backendfrompath("test.zarr"))
throw(ArgumentError("$path is not a Zarr dataset therefore we can't build the Pyramids inplace"))
end

Expand Down Expand Up @@ -527,4 +528,4 @@ end
include("broadcast.jl")


end
end
1 change: 1 addition & 0 deletions test/runtests.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ end
end

@testitem "Pyramid building RGB eltype" begin
using DimensionalData
using PyramidScheme: PyramidScheme as PS
using Colors
data = rand(RGB, 2000,2000)
Expand Down
Loading