Skip to content
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

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Add tilepyramids to PyramidScheme #49

wants to merge 11 commits into from

Conversation

meggart
Copy link
Member

@meggart meggart commented Aug 1, 2024

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.

Copy link

codecov bot commented Aug 1, 2024

Codecov Report

Attention: Patch coverage is 40.16393% with 73 lines in your changes missing coverage. Please review.

Project coverage is 57.88%. Comparing base (4bb73dd) to head (0f5ac9c).

Files Patch % Lines
src/pyramidprovider.jl 0.00% 28 Missing ⚠️
ext/PyramidSchemeTylerExt.jl 0.00% 23 Missing ⚠️
src/tilepyramids.jl 72.05% 19 Missing ⚠️
src/PyramidScheme.jl 0.00% 3 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

felixcremer

This comment was marked as outdated.

@felixcremer
Copy link
Member

We should also add some tests for this functionality. I took care of the compat entries already.

src/tilepyramids.jl Outdated Show resolved Hide resolved
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)
Copy link
Member

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?

Copy link
Member Author

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

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))
Copy link
Member

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.

Copy link
Member Author

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

@felixcremer
Copy link
Member

The eachchunk result of a Pyramid from the Openstreetmap providers seems wrong for me.
This is only for the base layer and I used the pyramid that is in the test that I am going to push.

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.
@felixcremer
Copy link
Member

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.

@felixcremer
Copy link
Member

plotting of a pyramid from a provider does not work.

This is because we don't handle additional dimensions not properly, but the rgb plotting does also not work.

@felixcremer
Copy link
Member

Could we also cache the Pyramid that we get from a provider?
I was playing around with it and it is very slow, because it seems as if it loads too much data.

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))
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants