Fix: Fix Shapely import#584
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes #583 by preventing geoalchemy2 from failing to import when Shapely is not installed, while keeping to_shape/from_shape guarded behind the optional dependency check.
Changes:
- Add a
TYPE_CHECKING-aware Shapely geometry type alias/fallback to avoid evaluatingshapely.*in annotations when Shapely isn’t installed. - Update
to_shape/from_shapetype annotations to use the new alias instead ofshapely.Geometry.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import shapely | ||
| import shapely.wkb | ||
| import shapely.wkt | ||
| from shapely import Geometry as ShapelyGeometry |
There was a problem hiding this comment.
from shapely import Geometry as ShapelyGeometry is likely incompatible with the Shapely versions this repo supports (tox installs Shapely>=1.3.0, and optional deps declare Shapely>=1.7). In Shapely 1.x there is no top-level Geometry, so this will raise ImportError even when Shapely is installed, incorrectly setting HAS_SHAPELY=False and breaking to_shape/from_shape. Import the geometry base type from shapely.geometry.base (e.g., BaseGeometry) or otherwise use a version-compatible symbol for the annotation without causing the Shapely import block to fail.
| from shapely import Geometry as ShapelyGeometry | |
| from shapely.geometry.base import BaseGeometry as ShapelyGeometry |
|
|
||
|
|
||
| @check_shapely() | ||
| def to_shape(element: WKBElement | WKTElement) -> shapely.Geometry: | ||
| def to_shape(element: WKBElement | WKTElement) -> ShapelyGeometry: | ||
| """Function to convert a :class:`geoalchemy2.types.SpatialElement` to a Shapely geometry. |
There was a problem hiding this comment.
This change fixes the runtime NameError when Shapely isn't installed, but there’s no test ensuring import geoalchemy2 (or import geoalchemy2.shape) works in an environment without Shapely. Consider adding a regression test that imports/reloads geoalchemy2.shape with Shapely unavailable (e.g., via an import hook or sys.modules patch) so this doesn’t regress again.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if not TYPE_CHECKING: | ||
|
|
||
| class ShapelyGeometry: | ||
| """Requires shapely. Install with: pip install geoalchemy2[shapely].""" |
There was a problem hiding this comment.
The if not TYPE_CHECKING: guard prevents the stub class from being defined during static type checking. When type checkers analyze code without shapely installed (or without access to shapely's type stubs), they will encounter a NameError for ShapelyGeometry in the function signatures.
Consider removing the if not TYPE_CHECKING: condition and defining the stub class unconditionally in the except block. This ensures the stub is available in both runtime and static analysis contexts.
| if not TYPE_CHECKING: | |
| class ShapelyGeometry: | |
| """Requires shapely. Install with: pip install geoalchemy2[shapely].""" | |
| class ShapelyGeometry: | |
| """Requires shapely. Install with: pip install geoalchemy2[shapely].""" |
Fixes #583