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

Added support for Proj6 and GDAL 3 #1700

Closed
wants to merge 5 commits into from
Closed

Added support for Proj6 and GDAL 3 #1700

wants to merge 5 commits into from

Conversation

normanb
Copy link
Contributor

@normanb normanb commented May 24, 2019

@sgillies I have added support for GDAL3 and Proj6. I have tested this with Python 3.6 and the CRS pytests.

@normanb
Copy link
Contributor Author

normanb commented May 25, 2019

@sgillies Some of the build errors are related to AWS secrets which I think is something you have to add. I see the GDAL 3.0 builds are missing proj6, I can look at the travis script for that.

sgillies
sgillies reviewed May 28, 2019
View reviewed changes
tests/test_crs.py
@@ -72,6 +72,7 @@ def test_read_epsg():
assert src.crs.to_epsg() == 32618


@requires_gdal_lt_3
Copy link
Member

Choose a reason for hiding this comment

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

👍

sgillies
sgillies reviewed May 28, 2019
View reviewed changes
tests/test_data_paths.py
from rasterio.rio.main import main_group


@requires_gdal_lt_3
Copy link
Member

@sgillies sgillies May 28, 2019 •
edited

Choose a reason for hiding this comment

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

We still need this test with GDAL 3. It confirms that the environment patching we do for wheels distributions will work. Same for the other three below.

sgillies
sgillies reviewed May 28, 2019
View reviewed changes
tests/test_data_paths.py
@@ -21,6 +23,7 @@ def test_gdal_data_find_file():
GDALDataFinder().find_file("header.dxf")


@requires_gdal_lt_3
Copy link
Member

Choose a reason for hiding this comment

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

In this case, we'll need to change the implementation for GDAL 3, and look for the new sqlite file in the wheel.

Also, setting PROJ_LIB is out of fashion with 3.0, there's a new function to do this: OSGeo/gdal#1191.

sgillies
sgillies requested changes May 28, 2019
View reviewed changes
Copy link
Member

@sgillies sgillies left a comment •
edited

Choose a reason for hiding this comment

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

@normanb a few changes will be needed yet. They're complicated.

I'm not sure what's up with the failing builds on Travis.

@normanb normanb force-pushed the gdal3 branch 3 times, most recently from 0b29205 to 62811bc Compare May 29, 2019 01:13
@normanb
Copy link
Contributor Author

normanb commented May 29, 2019

Looks like --with-proj isn't available in all of the GDAL builds, I don't want to use a static link (as that isn't the usual deployment), I have a local build of 2.2.4 that I will test with locally and update the build script.

@snowman2
Copy link
Member

I believe it used to be --with-proj4 in previous versions.

@sgillies
Copy link
Member

@normanb I recommend putting this work on hold. The wheel related issues are complicated and I'm extremely busy at the moment with little time for review. I was planning to work on support for 3.0 later in June.

@normanb
Copy link
Contributor Author

normanb commented May 29, 2019

@sgillies ok, I will fix the issue with proj configurations between gdal versions and park this, lmk when you want to pick this up in June and I may be able to work with you depending on my schedule. cc @jakebolewski

@normanb
Copy link
Contributor Author

normanb commented May 30, 2019 •
edited

The difference between GDAL versions is --with-static-proj4 or --with-proj I added a switch in the build script. However I think proj6 is a breaking change and may require a major version that isn't backwards compatible.

@vincentsarago
Copy link
Member

@normanb @sgillies I'd love to see this moving forward, is there anything I could do to help ?

@sgillies
Copy link
Member

sgillies commented Jul 3, 2019

@vincentsarago GDAL 3 isn't yet a must-have at work, so this issue gets bumped by higher priority tasks. One thing that would help: if rasterio power users would pick up some of the project support questions, i'd have more time to work on this feature.

@vincentsarago vincentsarago mentioned this pull request Jul 9, 2019
Closed
@rasterio rasterio deleted a comment from baerbock Jul 11, 2019
@rasterio rasterio deleted a comment from vincentsarago Jul 11, 2019
@sgillies
Copy link
Member

Heads up: I'm going to delete every single non-technical comment on this issue. If you're not a GDAL or Rasterio packager with experience building GDAL 3 and Proj 6, just sit tight.

@meyer1994 meyer1994 mentioned this pull request Jul 12, 2019
Open
@vincentsarago vincentsarago mentioned this pull request Jul 18, 2019
Closed
@sgillies sgillies mentioned this pull request Jul 22, 2019
Closed
@sgillies sgillies mentioned this pull request Jul 22, 2019
Merged
@sgillies
Copy link
Member

Superceded by #1729.

@sgillies sgillies closed this Jul 22, 2019
@sgillies sgillies mentioned this pull request Jul 29, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sgillies sgillies sgillies requested changes

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

Successfully merging this pull request may close these issues.

None yet
4 participants
@normanb @snowman2 @sgillies @vincentsarago