-
Notifications
You must be signed in to change notification settings - Fork 1
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 tilepyramids to PyramidScheme #49
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #49 +/- ##
==========================================
- Coverage 66.03% 57.88% -8.16%
==========================================
Files 3 6 +3
Lines 265 387 +122
==========================================
+ Hits 175 224 +49
- Misses 90 163 +73 ☔ View full report in Codecov by Sentry. |
We should also add some tests for this functionality. I took care of the compat entries already. |
src/tilepyramids.jl
Outdated
if mode === :band | ||
return MapTileDiskArray{rgbeltype(et),3,typeof(prov)}(prov, zoom, size(testtile, 1), nband) | ||
elseif mode === :rgb | ||
return MapTileDiskArray{et,2,typeof(prov)}(prov, zoom, size(testtile, 1), nband) |
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.
Shouldn't the number of bands be one in the case that the RGB info is in the color element type?
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.
Well there are no bands in this case, since the resulting array is 2d, and the field should never be accessed
src/tilepyramids.jl
Outdated
y1, y2 = first(ex1.Y), last(ex2.Y) | ||
stepx = (x2 - x1) / npix | ||
stepy = (y2 - y1) / npix | ||
x = X(range(x1 + stepx / 2, x2 - stepx / 2, length=npix)) |
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.
Should we add here that these are intervals? Because most likely in the tiles the value of a pixel should represent the average over the whole pixel area and not a point measurement.
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.
Yes, this would indeed be better, I will look into it
The eachchunk result of a Pyramid from the Openstreetmap providers seems wrong for me. julia> size(eachchunk(pyr))
(1, 5, 134217728)
julia> eachchunk(pyr)[1,1,1]
(1:3, 1:33333333, 1:1) |
this can still get some tests for the chunking structure.
plotting of a pyramid from a provider does not work. using GLMakie, PyramidScheme, TileProviders
julia> prov = TileProviders.OpenStreetMap(); pyr = Pyramid(prov)
julia> plot(pyr, colorbar=false)
ERROR: MethodError: no method matching defaultlimits(::Tuple{String, String}, ::typeof(identity))
Closest candidates are:
defaultlimits(::Tuple{Any, Any}, ::Any, ::Any)
@ Makie ~/.julia/packages/Makie/qMluh/src/makielayout/blocks/axis.jl:1418
defaultlimits(::Tuple{Nothing, Nothing}, ::Any)
@ Makie ~/.julia/packages/Makie/qMluh/src/makielayout/blocks/axis.jl:1429
defaultlimits(::Nothing, ::Any)
@ Makie ~/.julia/packages/Makie/qMluh/src/makielayout/blocks/axis.jl:1424
...
Stacktrace:
[1] defaultlimits(userlimits::Tuple{Tuple{String, String}, Tuple{Float64, Float64}}, xscale::Function, yscale::Function)
@ Makie ~/.julia/packages/Makie/qMluh/src/makielayout/blocks/axis.jl:1419
[2] initialize_block!(ax::Axis; palette::Nothing)
@ Makie ~/.julia/packages/Makie/qMluh/src/makielayout/blocks/axis.jl:158
[3] initialize_block!(ax::Axis)
@ Makie ~/.julia/packages/Makie/qMluh/src/makielayout/blocks/axis.jl:150
[4] _block(T::Type{…}, fig_or_scene::Figure, args::Vector{…}, kwdict::Dict{…}, bbox::Nothing; kwdict_complete::Bool)
@ Makie ~/.julia/packages/Makie/qMluh/src/makielayout/blocks.jl:371
[5] _block
@ ~/.julia/packages/Makie/qMluh/src/makielayout/blocks.jl:291 [inlined]
[6] #_block#1420
@ ~/.julia/packages/Makie/qMluh/src/makielayout/blocks.jl:259 [inlined]
[7] _block(::Type{Axis}, ::GridPosition; kwargs::@Kwargs{limits::Tuple{Tuple{…}, Tuple{…}}, aspect::DataAspect})
@ Makie ~/.julia/packages/Makie/qMluh/src/makielayout/blocks.jl:253
[8] _block
@ ~/.julia/packages/Makie/qMluh/src/makielayout/blocks.jl:246 [inlined]
[9] #_#1418
@ ~/.julia/packages/Makie/qMluh/src/makielayout/blocks.jl:237 [inlined]
[10] Block
@ ~/.julia/packages/Makie/qMluh/src/makielayout/blocks.jl:236 [inlined]
[11] plot(pyramid::Pyramid{…}; colorbar::Bool, kwargs::@Kwargs{})
@ PyramidScheme ~/Documents/PyramidScheme/src/PyramidScheme.jl:432
[12] top-level scope
@ REPL[64]:1
Some type information was truncated. Use `show(err)` to see complete types. |
This is because we don't handle additional dimensions not properly, but the rgb plotting does also not work. |
Could we also cache the Pyramid that we get from a provider? |
end | ||
z, x, y = tryparse.(Int, m.captures) | ||
any(isnothing, (x, y, z)) && return HTTP.Response(404, "Error: Malformed url") | ||
data = Tyler.fetch_tile(p, Tile(x, y, z)) |
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.
This fails, because Tyler is not available here.
This adds abstraction layers to treat any Pyramid as a TileProvider and also adds a
Pyramid
constructor to lazily expose a TileProvider as a Pyramid.