Opened 4 months ago
Last modified 5 days ago
#36441 assigned Cleanup/optimization
Add lazy loading support for GDAL library in `django.contrib.gis.gdal`
Reported by: | Josh Thomas | Owned by: | Josh Thomas |
---|---|---|---|
Component: | GIS | Version: | dev |
Severity: | Normal | Keywords: | gdal |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
I'd like to propose enabling lazy loading of the GDAL library in django.contrib.gis.gdal
, matching the existing behavior of django.contrib.gis.geos
with the GEOS library.
Currently, calling django.setup()
on projects using GeoDjango fails if GDAL is not installed, even when GDAL functionality is not being used. This creates an inconsistency in the GeoDjango framework where the GEOS library supports lazy loading (implemented ~10 years ago) but GDAL does not. The proposed change would implement lazy loading for the GDAL module similar to the existing GEOS implementation, ensuring the GDAL library is only loaded when actually accessed or used.
I have a working proof-of-concept in a branch on my fork of the Django repo ready for review (diff).
The implementation follows the same approach as the original GEOS lazy loading commit, wrapping the library loading in SimpleLazyObject
and converting function prototypes to use lazy loading. The public API remains unchanged (everything imported in django/contrib/gis/gdal/__init__.py
continues to work as before), though the imports related to the GDAL version from django.contrib.gis.gdal.libgdal
now use lazy evaluation. The existing test suite passes without modification, and I've added new tests specifically for the lazy loading behavior. Documentation updates have not been included yet as I wasn't sure if that was needed for a change like this.
I have created both a forum post and an issue on django/new-features. Both have only been open for a few days, so not a ton of time for people to see them. However, the initial reception has been positive, with the forum post receiving several likes and the new-features issue getting only thumbs-up reactions.
Change History (7)
comment:1 by , 4 months ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 4 months ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:4 by , 4 months ago
Needs documentation: | set |
---|
comment:5 by , 4 weeks ago
Needs documentation: | unset |
---|
comment:6 by , 4 weeks ago
I've updated the release note documentation updates from 6.0 to 6.1, as I assume due to the feature freeze this PR will not be making it in to 6.0.
comment:7 by , 5 days ago
Patch needs improvement: | set |
---|
I reviewed PR #19529 and ran the new GDAL lazy-loading tests locally. With PostGIS configured and GDAL missing, django.setup() no longer errors, and the tests in gis_tests.gdal_tests.tests pass.
There is a missing import in django/contrib/gis/gdal/driver.py:
NameError: name 'classproperty' is not defined
This can be fixed by adding:
from django.utils.functional import classproperty
Once that is corrected, the patch should be ready for checkin. Marking as Patch needs improvement until the import is added.
Thanks Josh, sounds good.
You said something about having a branch, so you can assign yourself and open a PR.