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

rasterio source module #1115

Merged
merged 97 commits into from
May 29, 2023
Merged

rasterio source module #1115

merged 97 commits into from
May 29, 2023

Conversation

banesullivan
Copy link
Contributor

@banesullivan banesullivan commented Apr 18, 2023

Supersedes #927 -- making PR from a dedicated branch in this repo to make my life easier

This branch implements a new tile source module based on rasterio to simplify the usage of geospatial tile sources where installing GDAL might not be easy.

This further refactors some of the existing GDAL and Mapnik tile sources so that they better share a base class of common geospatial code and similarly, the tests for these modules have been refactored to verify that all three source modules produce the same results.

  • Limitation: TILED outputs can only be done when the source is opened with a projection
  • Mapnik issue: see testPixel and testTileLinearStyleFromGeotiffs in test/test_source_mapnik.py. These are the only two remaining tests with differing results from the GDAL and rasterio sources. The styling for mapnik needs to be fixed (well maybe the styling should be fixed for all sources), then these tests should be removed to let the base class tests run.
  • rgba_geotiff.py: this is a pesky test file -- I will follow up in a separate PR with details on why. Currently only the GDAL source can handle this file and files like it.
  • Cross testing: I've cross tested with django-large-iamge to verify the GDAL source is still in working order and to verify the rasterio source works as expected (still missing TILED getRegion() support)
  • It's not recommended using the rasterio source without a projection -- though most things should work.

Still to address:

  • large_image.open() may not prefer the new rasterio source
  • Issues with CI testing (all passing locally)

Comment on lines +74 to +76

*.ipynb
NOTES.md
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are from my dev environment. Maybe remove?

@banesullivan
Copy link
Contributor Author

banesullivan commented May 26, 2023

Recommend merging as is. Though there will need to be follow-up work to:

  • More code cleanup
  • Ensuring large_image.open() properly chooses rasterio source (should always have lower priority than GDAL though)
  • Investigating slight differences in produced thumbnails/tiles -- I've managed to track down _applyStyle() as where the difference is introduced. Otherwise, rasterio and gdal are yielding the exact same data in getTile

Otherwise, this is generally complete

banesullivan and others added 5 commits May 26, 2023 17:05
Each source can define this independently, so that if only one
geospatial source is installed, it will still have the ability to
prioritize geospatial sources appropriately.
@banesullivan banesullivan merged commit 1b8b8f4 into master May 29, 2023
15 checks passed
@banesullivan banesullivan deleted the rasterio branch May 29, 2023 14:38
@banesullivan
Copy link
Contributor Author

@12rambau, thank you for getting this started!! You handled a ton of the legwork and made it possible for us to come in and finalize this. We'll tag a release and this should ship shortly.

@manthey
Copy link
Member

manthey commented May 29, 2023

🎉

@12rambau
Copy link
Contributor

amazing I was completly unable to finalizing it due to work constraints from my side, I'm super glad you made it !

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.

None yet

3 participants