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

DO NOT MERGE: Remove libgdal matrix, update to gdal 3.1 (take 2) #172

Closed
wants to merge 3 commits into from

Conversation

xylar
Copy link
Contributor

@xylar xylar commented Sep 8, 2020

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@xylar
Copy link
Contributor Author

xylar commented Sep 8, 2020

@conda-forge-admin, please rerender

@xylar
Copy link
Contributor Author

xylar commented Sep 8, 2020

Based on this comment from the developer: https://rasterio.groups.io/g/main/message/600, it seems like rasterio 1.1.5 will work with libgdal 3.1 and proj 7.1.0

I wasn't able to re-open #171 because the branch had been deleted so I'm opening a new PR.

@xylar
Copy link
Contributor Author

xylar commented Sep 8, 2020

@akrherz, many tests are failing with GDAL 3.1.2. If you want to dig into this and try to figure out why, that would be great. I don't really have time.

@akrherz
Copy link
Contributor

akrherz commented Sep 8, 2020

Thank you @xylar ! I am reviewing this all now.

@akrherz
Copy link
Contributor

akrherz commented Sep 8, 2020

It appears to me the test failures are just with test_transform_bounds_densify, which would likely be a proj issue? I see the tests ran against 7.1.0, I wonder if it is fixed in 7.1.1 ? This rasterio/rasterio#1987 PR appears to be passing checks with the GDAL 3.1 and Proj 7.1.1 combination?

@xylar
Copy link
Contributor Author

xylar commented Sep 8, 2020

We could test against proj 7.1.1 but until conda-forge updates to its pins to that version, it won't help because no other packages would be built with that version.

@xylar xylar changed the title Remove libgdal matrix, update to gdal 3.1 (take 2) DO NOT MERGE: Remove libgdal matrix, update to gdal 3.1 (take 2) Sep 8, 2020
@xylar
Copy link
Contributor Author

xylar commented Sep 8, 2020

Yeah, we can't even test with proj 7.1.1 yet because libgdal and possibly other dependencies haven't been built with that version.

@xylar
Copy link
Contributor Author

xylar commented Sep 8, 2020

We would need to do a test build of all of the dependencies that need proj first with proj 7.1.1 and then do a test build of this package with those dependencies. That's a lot more work than I'm willing to put in just to find out if this package might work later on when proj migrates to 7.1.1. For now, I think I (once agian) have to conclude that this version of rasterio is not compatible with the current pins and this should be closed.

But I'll leave it open until I get your agreement, @akrherz

@akrherz
Copy link
Contributor

akrherz commented Sep 8, 2020

That's reasonable, you have already gone way above and beyond here. Thanks again.

@xylar
Copy link
Contributor Author

xylar commented Sep 8, 2020

$ conda create -y -n test python=3.8 rasterio conda-tree
$  conda activate test
$ conda tree whoneeds proj
['geotiff', 'libspatialite', 'libgdal']

So you would need to do local builds of those three and rasterio itself with proj 7.1.1 if you wanted to see if this will work out. I can advise on how to do that but it's a lot of work.

@xylar xylar closed this Sep 8, 2020
@xylar xylar deleted the add_xylar branch September 8, 2020 13:46
@ocefpaf
Copy link
Member

ocefpaf commented Sep 8, 2020

I started a migration to the new proj 7.1.1. I would love to move to a x.x or even x pinning in proj but their history of breaking things is in micro releases is not encouraging.

@xylar
Copy link
Contributor Author

xylar commented Sep 8, 2020

I started a migration to the new proj 7.1.1.

Oh, that's great!

I would love to move to a x.x or even x pinning in proj but their history of breaking things is in micro releases is not encouraging.

Yeah, there's a tricky balance to be had between making things easier for us and breaking things. In the end, it seems like stricter pins are often worth the pain. It's such a mess to clean things up if the pin is too loose.

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.

4 participants