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.

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

Jump to bottom

Make GDALRasterSource behavior consistent with other RasterSources #3211

Merged
merged 3 commits into from
Mar 27, 2020
Merged

Make GDALRasterSource behavior consistent with other RasterSources #3211

merged 3 commits into from
Mar 27, 2020

Conversation

pomadchin
Copy link
Member

@pomadchin pomadchin commented Mar 26, 2020 •
edited
Loading

Overview

This PR:

  • Adds the base cellSize to the list of resolutions to behave similar to other RasterSources.
  • Bumps GDALWarp version up to 1.0.0
  • Fixes Jackson dependencies

This issues makes GDALRasterSource, GeoTiffRasterSource and DEMRasterSource to behave more consistent.

Checklist

Related to #3210
Closes #3212

@pomadchin pomadchin requested a review from echeipesh March 26, 2020 18:44
@pomadchin pomadchin force-pushed the fix/rastersources-reoslution branch 3 times, most recently from edd154c to f059799 Compare March 26, 2020 19:48
echeipesh
echeipesh approved these changes Mar 26, 2020
View reviewed changes
Copy link
Contributor

@echeipesh echeipesh left a comment

Choose a reason for hiding this comment

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

LGTM, does this really close the issue for .resolutions being consistent for all instances though?

@pomadchin
Copy link
Member Author

pomadchin commented Mar 26, 2020 •
edited
Loading

@echeipesh at least GeoTiffRasterSource, GDALRasterSource and DEMRasterSource behave the same way. I'm wondering about the GeoTrellisRasterSource, it seems to me that instead of a base cellSize it can use the current.

@pomadchin
* Make GDALRasterSource behavior consistent with other RasterSources
a4530e2 
* GDALRasterSource.resample should take into account the resampleMethod
@pomadchin pomadchin force-pushed the fix/rastersources-reoslution branch from f059799 to a4530e2 Compare March 26, 2020 20:06
@pomadchin pomadchin force-pushed the fix/rastersources-reoslution branch from 6bf87a2 to 3c9ac7d Compare March 26, 2020 22:12
@pomadchin pomadchin mentioned this pull request Mar 26, 2020
Closed
@pomadchin
Copy link
Member Author

Will merge this issue once travis is happy.

The further discussion and thoughts that I came up with are here #3210 (comment)

@pomadchin pomadchin force-pushed the fix/rastersources-reoslution branch from 3c9ac7d to 27d5c1f Compare March 26, 2020 22:17
@pomadchin pomadchin force-pushed the fix/rastersources-reoslution branch from 27d5c1f to 61b0f86 Compare March 26, 2020 22:19
@pomadchin pomadchin merged commit 9c3d159 into locationtech:master Mar 27, 2020
@pomadchin pomadchin deleted the fix/rastersources-reoslution branch March 27, 2020 01:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@echeipesh echeipesh echeipesh approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Fix Jackson dependencies
2 participants
@pomadchin @echeipesh