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

Replace Stamen tiles + fix other deprecations #330

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

AmitAronovitch
Copy link
Contributor

@AmitAronovitch AmitAronovitch commented Jan 4, 2025

Resolves #329

This PR is for making telluric work with more up-to-date versions python and packages.

Changes included so far:

  1. Use CartoDB refernce maps instead of Stamen (which are not available any more - see folium #1803
  2. Remove dependency on deprecated pkg_resources (not required for Python>=3.7)
  3. Fix some failing tests and linters

@matiasg
Copy link

matiasg commented Jan 23, 2025

Hi, @AmitAronovitch !!
Are you still using telluric? Do you want me to try find somebody to review the PR?

@AmitAronovitch
Copy link
Contributor Author

Hi, @AmitAronovitch !! Are you still using telluric? Do you want me to try find somebody to review the PR?

Hi @matiasg , yes that would be great :-)

Note however that releasing a new version of telluric still requires some work.

This is because the dependencies (mainly rasterio) had moved forwards and we need to change stuff if we want to telluric to use supported versions of its dependencies...

My branch fixes some issues, but there are some failing tests which I am not sure what to do with (it seems to be some regression test related to fixup of rounding issues at the edges of windows. It breaks because rasterio had changed the way it rounds up its images, but I do not have the rasters that the old telluric issue was trying to fix, so cannot verify that fixing the code does not re-break them).
I will add more info on this later.

@enomis-dev
Copy link
Contributor

Thanks for sharing @AmitAronovitch , i'll take a look asap

@AmitAronovitch
Copy link
Contributor Author

Hi @enomis-dev ,

Adding some info regarding the failing test (there might be some more...)

The failing test is tests/test_georaster.py::test_crop_respects_rounding_precision (dump below),

This test seems to be a regression test, that @drnextgis had added after fixing some issue (probably gaps that used to happen at the edges of tiles). The comment refers to #311.
That PR had added precision=3 to the round_offsets() call. However, this parameter is ignored in more recent versions of rasterio. See this PR: rasterio/rasterio#2311, which was merged in rasterio 1.3 (around the same time of #311 - it seems like rasterio devs were dealing with similar issues to ours, but 1.3 was not released yet).

I do not know if running with newer rasterio will re-introduce the original problem of #311 or not, because the test does not test it directly (it uses dummy rasters, and only compares window sizes - not checking effects at the edges of tiled rasters, which I believe the original problem was).

>       assert r1_cropped.shape == r2_cropped.shape
E       assert (1, 2663, 2180) == (1, 2663, 2179)
E         
E         At index 2 diff: 2180 != 2179
E         
E         Full diff:
E           (
E               1,
E               2663,
E         -     2179,
E         ?       ^^
E         +     2180,
E         ?       ^^
E           )

And here are some details from the debugger:

(Pdb) p roi
GeoVector(shape=POLYGON ((271806.0179640717 1438839.4164977344, 271806.0179640717 1519170.2272236191, 337216.40041283204 1519170.2272236191, 337216.40041283204 1438839.4164977344, 271806.0179640717 1438839.4164977344)), crs=EPSG:32628)
(Pdb) p r1.image.shape
(1, 4289, 3517)
(Pdb) p r1.affine
Affine(30.0, 0.0, 231681.0,
       0.0, -30.0, 1567951.0)
(Pdb) p r2.image.shape
(1, 4289, 3517)
(Pdb) p r2.affine
Affine(30.0, 0.0, 231669.0,
       0.0, -30.0, 1567970.0)

first case in crop:

(Pdb) p bounds, window
((1337, 1626, 3517, 4289), Window(col_off=1337, row_off=1626, width=2180, height=2663))

second case:

((1338, 1626, 3517, 4289), Window(col_off=1338, row_off=1626, width=2179, height=2663))

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.

reference maps not working anymore
3 participants