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

Support GDAL 3+ #27

Closed
2 tasks done
dcdenu4 opened this issue Feb 20, 2020 · 4 comments
Closed
2 tasks done

Support GDAL 3+ #27

dcdenu4 opened this issue Feb 20, 2020 · 4 comments
Assignees
Labels
resolved The task has been resolved

Comments

@dcdenu4
Copy link
Member

dcdenu4 commented Feb 20, 2020

InVEST tests currently fail when using GDAL 3+. This is mostly related to how GDAL 3 has changed how it handles spatial references, and particularly coordinate transformations. This issue should be blocked on the Pygeoprocessing version of this issue. The main consideration is whether we should support both GDAL 2 and GDAL 3 or move on to GDAL 3 and drop support for GDAL 2. The main thing here is that there are slight variations in spatial coordinates pertaining to bounding boxes and extents. This would mean that in certain cases we would have to carry around expected test data for both GDAL 2 runs and GDAL 3 runs. That is doable, but perhaps not ideal?

  • Pygeoprocessing supports GDAL 3+
  • Update history with changes
@dcdenu4 dcdenu4 self-assigned this Feb 20, 2020
dcdenu4 added a commit to dcdenu4/invest that referenced this issue Feb 21, 2020
Updating requirements.txt to look for versions equal to gdal
3 or greater. Updating Makefile to use new revision of test
data which includes changes to crop production tests. These
tests are slightly different because gdal 3 handles spatial
references different and there is a slight variation in extents.
dcdenu4 added a commit to dcdenu4/invest that referenced this issue Feb 21, 2020
Adding coordinate transformation function to utils.py so that all InVEST
models that handle their own transformations can now have a single
entry point for it. This function is compatible with gdal 3 but
NOT gdal 2. This function sets unprojected spatial references to
be "GIS Friendly", meaning use lon,lat when transforming points.

Adding tests for this new function.
dcdenu4 added a commit to dcdenu4/invest that referenced this issue Feb 21, 2020
Use utils transformation function and remove private function for
doing coordinate transforms
dcdenu4 added a commit to dcdenu4/invest that referenced this issue Feb 21, 2020
Switching to use the utils function for handling coordinate
transformations
dcdenu4 added a commit to dcdenu4/invest that referenced this issue Feb 21, 2020
GDAL 3 doesn't support built in next() iterator on layers anymore,
so now calling layer.GetNextFeature() instead.
dcdenu4 added a commit to dcdenu4/invest that referenced this issue Feb 21, 2020
Using utils coordinate transform function.
dcdenu4 added a commit to dcdenu4/invest that referenced this issue Feb 21, 2020
Changing the last manual coordinate transformation to use utils
dcdenu4 added a commit to dcdenu4/invest that referenced this issue Feb 28, 2020
Removing conditionals around setting axis order, as they are not
actually needed. GDAL 3 will properly set axis orders on the CRS if
geographic or projected.

Exposing the axis order with a constant at the top of utils that is used
and set to a default in def create_coordinate_transformation
@phargogh
Copy link
Member

Also, when we have a functioning binary build based on GDAL3, let's be sure to have Jade and/or Stacie test the new build on the MAR datasets that were having LZW errors! (related to #29 )

@phargogh
Copy link
Member

Just heard from Jade that the latest release (InVEST 3.8.1, which uses GDAL 2.4.4), resolves the LZW issue which confirms our hypothesis about the compression issue being related to OSGeo/gdal#1439. The upgrade to GDAL 3 should also contain this fix.

@dcdenu4
Copy link
Member Author

dcdenu4 commented May 19, 2020

This is dependent on Pygeoprocessing milestone 2.0

@dcdenu4
Copy link
Member Author

dcdenu4 commented Jun 23, 2020

closed from #150

@dcdenu4 dcdenu4 closed this as completed Jun 23, 2020
@dcdenu4 dcdenu4 added the resolved The task has been resolved label Jun 23, 2020
emlys added a commit to emlys/invest that referenced this issue Apr 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
resolved The task has been resolved
Projects
None yet
Development

No branches or pull requests

2 participants