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

Change dependency from libgdal to gdal #49

Closed
wants to merge 2 commits into from
Closed

Change dependency from libgdal to gdal #49

wants to merge 2 commits into from

Conversation

theroggy
Copy link
Contributor

@theroggy theroggy commented Jul 26, 2024 •
edited
Loading

GDAL recently reorganised its conda packages as discussed in conda-forge/gdal-feedstock#722. In short: the libgdal package was split up in a core package, libgdal-core, and a list of seperate packages for many GDAL plugins, to be able to avoid the (large) footprint of the entire libgdal. For pyogrio this is also very relevant, as it depends on libgdal and most pyogrio users only need libgdal-core.

However, as libgdal-core only exists for GDAL 3.9.1, just depending on libgdal-core would mean - as far as I know - that users of future versions of pyogrio could only install that new version in combination with GDAL 3.9.1, not with older versions of GDAL, as for those only the full libgdal package exists.

Because of this, this PR at the moment "proposes" to changed the dependency to the gdal package. This is the GDAL package with python bindings (which aren't needed for pyogrio), but the advantage is that for old versions of GDAL this depends on libgdal, but starting from GDAL 3.9.1 it depends on libgdal-core, so it can offer a workaround for the issue above.

If conda would offer an option to depend on libgdal-core, but if not available (for the needed version of GDAL), fallback to libgdal, that would be even better.

Closes #48.

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-webservices
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/meta.yaml) and found it was in an excellent condition.

@theroggy
Copy link
Contributor Author

@conda-forge-admin please rerender

@theroggy
Copy link
Contributor Author

theroggy commented Jul 26, 2024 •
edited
Loading

@ocefpaf as described in the PR description above, we wonder if conda offers an option to depend on libgdal-core, but if (another dependency in) the conda environment needs an older version of GDAL, it would fallback to libgdal. Could you shed some light, or is depending on the gdal package the way to go?

@ocefpaf
Copy link
Member

ocefpaf commented Jul 26, 2024

@ocefpaf as described in the PR description above, we wonder if conda offers an option to depend on libgdal-core, but if (another dependency in) the conda environment needs an older version of GDAL, it would fallback to libgdal. Could you shed some light, or is depending on the gdal package the way to go?

If something requests older GDAL, an older build of pyogrio would also be selected. If you want to keep that as an option for future versions, then a variant would need to be built. IMO it is not worth the effort. Even more so given that the current pyogrio will be available for both for a while if we merge this one.

With that said. you are depending on gdal only here, that means conda will solve for whatever libgdal(-core or not) the gdal version requested is available.

Does that make sense?

@theroggy
Copy link
Contributor Author

theroggy commented Jul 26, 2024 •
edited
Loading

If something requests older GDAL, an older build of pyogrio would also be selected. If you want to keep that as an option for future versions, then a variant would need to be built. IMO it is not worth the effort. Even more so given that the current pyogrio will be available for both for a while if we merge this one.

With that said. you are depending on gdal only here, that means conda will solve for whatever libgdal(-core or not) the gdal version requested is available.

Does that make sense?

Yes. This is indeed the reason I created this PR to depend on gdal. I expected this to be the only/best (practical) solution to be able to have recent versions of pyogrio use libgdal-core for new GDAL versions and libgdal for older GDAL versions.

Based on your answer, I think you agree?

@jorisvandenbossche
Copy link
Member

But (and that's something I forgot when we chatted about this in our meeting), pyogrio binaries are built against a specific version of GDAL anyway. So it's not actually possible to install the latest pyogrio (assume the resulting binary of this rerendering) with an older version of GDAL.
In that case the concern is a bit moot and it is actually fine to just directly depend on libgdal-core?

@theroggy
Copy link
Contributor Author

Yes, indeed... in the corresponding rasterio issue I created (conda-forge/rasterio-feedstock#303) this also was just mentioned...

I already encountered the case that with global pinning a rebuild was needed to use the latest version, but apparently the pinning is really on a specific version...
Kind of a weird situation, as in pyogrio we try to support keep support older gdal versions as well and the GDAL C API is binary compatible as far as I know...

@theroggy
Copy link
Contributor Author

theroggy commented Jul 27, 2024 •
edited
Loading

Another point raised in the rasterio topic: maybe better to wait for the next version (0.10 for pyogrio) as it can be seen as it can break an installation, even if it is easy to solve it?

@theroggy theroggy marked this pull request as draft July 27, 2024 10:03
@olivier-lacroix
Copy link

Hello there, I am looking forward to this change being merged to slim down our environments. I see very few, if any, open issues are linked to the 0.10 milestone: is there any target release date for 0.10 and the change in feedstock?

@jorisvandenbossche
Copy link
Member

@olivier-lacroix indeed, we hope to release shortly. I opened an issue to track the release at geopandas/pyogrio#470

@olivier-lacroix
Copy link

Thanks @jorisvandenbossche ! Looking forward to it!

@theroggy theroggy mentioned this pull request Sep 29, 2024
Merged
3 tasks
@theroggy theroggy closed this Oct 1, 2024
@theroggy theroggy deleted the Change-dependency-from-libgdal-to-gdal branch October 1, 2024 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brendan-ward brendan-ward Awaiting requested review from brendan-ward brendan-ward is a code owner

@jorisvandenbossche jorisvandenbossche Awaiting requested review from jorisvandenbossche jorisvandenbossche is a code owner

@martinfleis martinfleis Awaiting requested review from martinfleis martinfleis is a code owner

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

Successfully merging this pull request may close these issues.

change dependency from libgdal to libgdal-core or gdal
4 participants
@theroggy @ocefpaf @jorisvandenbossche @olivier-lacroix