From 519e09d3a34ca46452cee98d779a0fd7ce8a0f18 Mon Sep 17 00:00:00 2001 From: James Parrott <80779630+JamesParrott@users.noreply.github.com> Date: Sun, 27 Jul 2025 18:58:53 +0100 Subject: [PATCH 1/6] Simplify Writer.multipatch type sig, and make more general --- src/shapefile.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shapefile.py b/src/shapefile.py index 0c1b401..4dd1aee 100644 --- a/src/shapefile.py +++ b/src/shapefile.py @@ -2865,7 +2865,7 @@ def polyz(self, polys: list[Points]): shapeType = POLYGONZ self._shapeparts(parts=polys, shapeType=shapeType) - def multipatch(self, parts: list[list[PointZ]], partTypes: list[int]): + def multipatch(self, parts: list[Points], partTypes: list[int]): """Creates a MULTIPATCH shape. Parts is a collection of 3D surface patches, each made up of a list of xyzm values. PartTypes is a list of types that define each of the surface patches. From cf217363b565bfd398904a442066c55324e4f199 Mon Sep 17 00:00:00 2001 From: James Parrott <80779630+JamesParrott@users.noreply.github.com> Date: Sun, 27 Jul 2025 19:19:54 +0100 Subject: [PATCH 2/6] Type hint Writer._shapeparts --- src/shapefile.py | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/shapefile.py b/src/shapefile.py index 4dd1aee..2aedbe4 100644 --- a/src/shapefile.py +++ b/src/shapefile.py @@ -2793,14 +2793,14 @@ def pointz(self, x: float, y: float, z: float = 0.0, m: Optional[float] = None): pointShape.points.append((x, y, z, m)) self.shape(pointShape) - def multipoint(self, points: Coords): + def multipoint(self, points: Points): """Creates a MULTIPOINT shape. Points is a list of xy values.""" shapeType = MULTIPOINT # nest the points inside a list to be compatible with the generic shapeparts method self._shapeparts(parts=[points], shapeType=shapeType) - def multipointm(self, points: list[PointM]): + def multipointm(self, points: Points): """Creates a MULTIPOINTM shape. Points is a list of xym values. If the m (measure) value is not included, it defaults to None (NoData).""" @@ -2808,7 +2808,7 @@ def multipointm(self, points: list[PointM]): # nest the points inside a list to be compatible with the generic shapeparts method self._shapeparts(parts=[points], shapeType=shapeType) - def multipointz(self, points): + def multipointz(self, points: Points): """Creates a MULTIPOINTZ shape. Points is a list of xyzm values. If the z (elevation) value is not included, it defaults to 0. @@ -2817,7 +2817,7 @@ def multipointz(self, points): # nest the points inside a list to be compatible with the generic shapeparts method self._shapeparts(parts=[points], shapeType=shapeType) - def line(self, lines: list[Coords]): + def line(self, lines: list[Points]): """Creates a POLYLINE shape. Lines is a collection of lines, each made up of a list of xy values.""" shapeType = POLYLINE @@ -2838,7 +2838,7 @@ def linez(self, lines: list[Points]): shapeType = POLYLINEZ self._shapeparts(parts=lines, shapeType=shapeType) - def poly(self, polys: list[Coords]): + def poly(self, polys: list[Points]): """Creates a POLYGON shape. Polys is a collection of polygons, each made up of a list of xy values. Note that for ordinary polygons the coordinates must run in a clockwise direction. @@ -2891,7 +2891,7 @@ def multipatch(self, parts: list[Points], partTypes: list[int]): # write the shape self.shape(polyShape) - def _shapeparts(self, parts, shapeType): + def _shapeparts(self, parts: list[Points], shapeType: int): """Internal method for adding a shape that has multiple collections of points (parts): lines, polygons, and multipoint shapes. """ @@ -2908,10 +2908,11 @@ def _shapeparts(self, parts, shapeType): # set part index position polyShape.parts.append(len(polyShape.points)) # add points - for point in part: - # Ensure point is list - point_list = list(point) - polyShape.points.append(point_list) + # for point in part: + # # Ensure point is list + # point_list = list(point) + # polyShape.points.append(point_list) + polyShape.points.extend(part) # write the shape self.shape(polyShape) From b8aa53997b089a1bf8cb8abb09c87be66031fad4 Mon Sep 17 00:00:00 2001 From: James Parrott <80779630+JamesParrott@users.noreply.github.com> Date: Mon, 28 Jul 2025 09:31:05 +0100 Subject: [PATCH 3/6] Update shapefile.py --- src/shapefile.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shapefile.py b/src/shapefile.py index 2aedbe4..6ce0cc7 100644 --- a/src/shapefile.py +++ b/src/shapefile.py @@ -111,7 +111,7 @@ PointM = tuple[float, float, Optional[float]] PointZ = tuple[float, float, float, Optional[float]] -Coord = Union[Point2D, Point2D, Point3D] +Coord = Union[Point2D, Point3D] Coords = list[Coord] Point = Union[Point2D, PointM, PointZ] From 8051e48335fa7653e4c6385960df7984efa6150d Mon Sep 17 00:00:00 2001 From: James Parrott <80779630+JamesParrott@users.noreply.github.com> Date: Mon, 28 Jul 2025 10:41:40 +0100 Subject: [PATCH 4/6] Attempt 2 at typing GeoJSON --- src/shapefile.py | 75 ++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 70 insertions(+), 5 deletions(-) diff --git a/src/shapefile.py b/src/shapefile.py index 6ce0cc7..02e8ab6 100644 --- a/src/shapefile.py +++ b/src/shapefile.py @@ -28,11 +28,13 @@ Generic, Iterable, Iterator, + Literal, NoReturn, Optional, Protocol, Reversible, Sequence, + TypedDict, TypeVar, Union, overload, @@ -143,6 +145,69 @@ class HasGeoInterface(Protocol): @property def __geo_interface__(self) -> Any: ... +class GeoJSONPoint(TypedDict): + type: Literal["Point"] + # We fix to a tuple (to statically check the length is 2, 3 or 4) but + # RFC7946 only requires: "A position is an array of numbers. There MUST be two or more + # elements. " + # RFC7946 also requires long/lat easting/northing which we do not enforce, + # and despite the SHOULD NOT, we may use a 4th element for Shapefile M Measures. + coordinates: Point + +class GeoJSONMultiPoint(TypedDict): + type: Literal["MultiPoint"] + coordinates: Points + +class GeoJSONLineString(TypedDict): + type: Literal["LineString"] + # "Two or more positions" not enforced by type checker + # https://datatracker.ietf.org/doc/html/rfc7946#section-3.1.4 + coordinates: Points + +class GeoJSONMultiLineString(TypedDict): + type: Literal["MultiLineString"] + coordinates: list[Points] + +class GeoJSONPolygon(TypedDict): + type: Literal["Polygon"] + # Other requirements for Polygon not enforced by type checker + # https://datatracker.ietf.org/doc/html/rfc7946#section-3.1.6 + coordinates: list[Points] + +class GeoJSONMultiPolygon(TypedDict): + type: Literal["MultiPolygon"] + coordinates: list[list[Points]] + +GeoJSONHomogeneousGeometryObject = Union[ + GeoJSONPoint, GeoJSONMultiPoint, + GeoJSONLineString, GeoJSONMultiLineString, + GeoJSONPolygon, GeoJSONMultiPolygon, +] + +class GeoJSONGeometryCollection(TypedDict): + type: Literal["GeometryCollection"] + geometries: list[GeoJSONHomogeneousGeometryObject] + +# RFC7946 3.1 +GeoJSONObject = Union[GeoJSONHomogeneousGeometryObject, GeoJSONGeometryCollection] + +class GeoJSONFeature(TypedDict): + type: Literal["Feature"] + properties: Optional[dict[str, Any]] # RFC7946 3.2 "(any JSON object or a JSON null value)" + geometry: Optional[GeoJSONObject] + + +class GeoJSONFeatureCollection(TypedDict, total= False): + type: Literal["FeatureCollection"] + features: list[GeoJSONFeature] + # bbox is optional + # typing.NotRequired requires Python 3.11 + # and we must support 3.9 (at least until October) + # https://docs.python.org/3/library/typing.html#typing.Required + # Is there a backport? + bbox: list[float] + + # Helpers @@ -541,7 +606,7 @@ def __init__( # self.bbox: Optional[_Array[float]] = None @property - def __geo_interface__(self): + def __geo_interface__(self) -> GeoJSONHomogeneousGeometryObject: if self.shapeType in [POINT, POINTM, POINTZ]: # point if len(self.points) == 0: @@ -922,7 +987,7 @@ def __init__(self, shape: Optional[Shape] = None, record: Optional[_Record] = No self.record = record @property - def __geo_interface__(self): + def __geo_interface__(self) -> GeoJSONFeature: return { "type": "Feature", "properties": self.record.as_dict(date_strings=True), @@ -942,7 +1007,7 @@ def __repr__(self): return f"Shapes: {list(self)}" @property - def __geo_interface__(self): + def __geo_interface__(self) -> GeoJSONGeometryCollection: # Note: currently this will fail if any of the shapes are null-geometries # could be fixed by storing the shapefile shapeType upon init, returning geojson type with empty coords collection = { @@ -962,7 +1027,7 @@ def __repr__(self): return f"ShapeRecords: {list(self)}" @property - def __geo_interface__(self): + def __geo_interface__(self) -> GeoJSONFeatureCollection: collection = { "type": "FeatureCollection", "features": [shaperec.__geo_interface__ for shaperec in self], @@ -1284,7 +1349,7 @@ def __iter__(self): yield from self.iterShapeRecords() @property - def __geo_interface__(self): + def __geo_interface__(self) -> GeoJSONFeatureCollection: shaperecords = self.shapeRecords() fcollection = shaperecords.__geo_interface__ fcollection["bbox"] = list(self.bbox) From 2c66018b9a7dce3b9088b7b8783a85c90c9b06eb Mon Sep 17 00:00:00 2001 From: James Parrott <80779630+JamesParrott@users.noreply.github.com> Date: Mon, 28 Jul 2025 11:21:32 +0100 Subject: [PATCH 5/6] Refine and relax typed dicts as needs arise --- src/shapefile.py | 42 +++++++++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/src/shapefile.py b/src/shapefile.py index 02e8ab6..9488e69 100644 --- a/src/shapefile.py +++ b/src/shapefile.py @@ -152,7 +152,7 @@ class GeoJSONPoint(TypedDict): # elements. " # RFC7946 also requires long/lat easting/northing which we do not enforce, # and despite the SHOULD NOT, we may use a 4th element for Shapefile M Measures. - coordinates: Point + coordinates: Union[Point, tuple[()]] class GeoJSONMultiPoint(TypedDict): type: Literal["MultiPoint"] @@ -197,9 +197,11 @@ class GeoJSONFeature(TypedDict): geometry: Optional[GeoJSONObject] -class GeoJSONFeatureCollection(TypedDict, total= False): +class GeoJSONFeatureCollection(TypedDict): type: Literal["FeatureCollection"] features: list[GeoJSONFeature] + +class GeoJSONFeatureCollectionWithBBox(GeoJSONFeatureCollection, total=False): # bbox is optional # typing.NotRequired requires Python 3.11 # and we must support 3.9 (at least until October) @@ -990,14 +992,16 @@ def __init__(self, shape: Optional[Shape] = None, record: Optional[_Record] = No def __geo_interface__(self) -> GeoJSONFeature: return { "type": "Feature", - "properties": self.record.as_dict(date_strings=True), + "properties": None + if self.record is None + else self.record.as_dict(date_strings=True), "geometry": None - if self.shape.shapeType == NULL + if self.shape is None or self.shape.shapeType == NULL else self.shape.__geo_interface__, } -class Shapes(list): +class Shapes(list[Optional[Shape]]): """A class to hold a list of Shape objects. Subclasses list to ensure compatibility with former work and to reuse all the optimizations of the builtin list. In addition to the list interface, this also provides the GeoJSON __geo_interface__ @@ -1010,14 +1014,17 @@ def __repr__(self): def __geo_interface__(self) -> GeoJSONGeometryCollection: # Note: currently this will fail if any of the shapes are null-geometries # could be fixed by storing the shapefile shapeType upon init, returning geojson type with empty coords - collection = { - "type": "GeometryCollection", - "geometries": [shape.__geo_interface__ for shape in self], - } + collection = GeoJSONGeometryCollection( + type= "GeometryCollection", + geometries = [shape.__geo_interface__ + for shape in self + if shape is not None + ], + ) return collection -class ShapeRecords(list): +class ShapeRecords(list[ShapeRecord]): """A class to hold a list of ShapeRecord objects. Subclasses list to ensure compatibility with former work and to reuse all the optimizations of the builtin list. In addition to the list interface, this also provides the GeoJSON __geo_interface__ @@ -1030,9 +1037,12 @@ def __repr__(self): def __geo_interface__(self) -> GeoJSONFeatureCollection: collection = { "type": "FeatureCollection", - "features": [shaperec.__geo_interface__ for shaperec in self], + "features": [] #shaperec.__geo_interface__ for shaperec in self], } - return collection + return GeoJSONFeatureCollection( + type="FeatureCollection", + features=[shaperec.__geo_interface__ for shaperec in self], + ) class ShapefileException(Exception): @@ -1349,10 +1359,12 @@ def __iter__(self): yield from self.iterShapeRecords() @property - def __geo_interface__(self) -> GeoJSONFeatureCollection: + def __geo_interface__(self) -> GeoJSONFeatureCollectionWithBBox: shaperecords = self.shapeRecords() - fcollection = shaperecords.__geo_interface__ - fcollection["bbox"] = list(self.bbox) + fcollection = GeoJSONFeatureCollectionWithBBox( + bbox = list(self.bbox), + **shaperecords.__geo_interface__, + ) return fcollection @property From aca60cf50bc83d70747b21e1bcd2e40a60796651 Mon Sep 17 00:00:00 2001 From: James Parrott <80779630+JamesParrott@users.noreply.github.com> Date: Mon, 28 Jul 2025 11:40:03 +0100 Subject: [PATCH 6/6] Typehint all __geo_interfaces__ (mostly, but not entirely GeoJSON compliant) --- .../workflows/run_checks_build_and_test.yml | 2 +- src/shapefile.py | 77 +++++++++++-------- 2 files changed, 44 insertions(+), 35 deletions(-) diff --git a/.github/workflows/run_checks_build_and_test.yml b/.github/workflows/run_checks_build_and_test.yml index f9e4b2e..89d8c20 100644 --- a/.github/workflows/run_checks_build_and_test.yml +++ b/.github/workflows/run_checks_build_and_test.yml @@ -26,7 +26,7 @@ jobs: python -m pip install --upgrade pip pip install pytest pylint pylint-per-file-ignores pip install -e . - - name: run Pylint for errors and warnings only + - name: run Pylint for errors, warnings and remarks only (ignore Comments/ Code style) run: | pylint --disable=C test_shapefile.py src/shapefile.py diff --git a/src/shapefile.py b/src/shapefile.py index 9488e69..8008fe1 100644 --- a/src/shapefile.py +++ b/src/shapefile.py @@ -145,55 +145,70 @@ class HasGeoInterface(Protocol): @property def __geo_interface__(self) -> Any: ... + class GeoJSONPoint(TypedDict): type: Literal["Point"] - # We fix to a tuple (to statically check the length is 2, 3 or 4) but + # We fix to a tuple (to statically check the length is 2, 3 or 4) but # RFC7946 only requires: "A position is an array of numbers. There MUST be two or more # elements. " # RFC7946 also requires long/lat easting/northing which we do not enforce, # and despite the SHOULD NOT, we may use a 4th element for Shapefile M Measures. - coordinates: Union[Point, tuple[()]] - + coordinates: Union[Point, tuple[()]] + + class GeoJSONMultiPoint(TypedDict): type: Literal["MultiPoint"] coordinates: Points + class GeoJSONLineString(TypedDict): type: Literal["LineString"] # "Two or more positions" not enforced by type checker # https://datatracker.ietf.org/doc/html/rfc7946#section-3.1.4 coordinates: Points - + + class GeoJSONMultiLineString(TypedDict): type: Literal["MultiLineString"] coordinates: list[Points] + class GeoJSONPolygon(TypedDict): type: Literal["Polygon"] - # Other requirements for Polygon not enforced by type checker + # Other requirements for Polygon not enforced by type checker # https://datatracker.ietf.org/doc/html/rfc7946#section-3.1.6 coordinates: list[Points] + class GeoJSONMultiPolygon(TypedDict): type: Literal["MultiPolygon"] coordinates: list[list[Points]] + GeoJSONHomogeneousGeometryObject = Union[ - GeoJSONPoint, GeoJSONMultiPoint, - GeoJSONLineString, GeoJSONMultiLineString, - GeoJSONPolygon, GeoJSONMultiPolygon, + GeoJSONPoint, + GeoJSONMultiPoint, + GeoJSONLineString, + GeoJSONMultiLineString, + GeoJSONPolygon, + GeoJSONMultiPolygon, ] + class GeoJSONGeometryCollection(TypedDict): type: Literal["GeometryCollection"] geometries: list[GeoJSONHomogeneousGeometryObject] + # RFC7946 3.1 GeoJSONObject = Union[GeoJSONHomogeneousGeometryObject, GeoJSONGeometryCollection] + class GeoJSONFeature(TypedDict): type: Literal["Feature"] - properties: Optional[dict[str, Any]] # RFC7946 3.2 "(any JSON object or a JSON null value)" + properties: Optional[ + dict[str, Any] + ] # RFC7946 3.2 "(any JSON object or a JSON null value)" geometry: Optional[GeoJSONObject] @@ -201,16 +216,16 @@ class GeoJSONFeatureCollection(TypedDict): type: Literal["FeatureCollection"] features: list[GeoJSONFeature] + class GeoJSONFeatureCollectionWithBBox(GeoJSONFeatureCollection, total=False): # bbox is optional - # typing.NotRequired requires Python 3.11 + # typing.NotRequired requires Python 3.11 # and we must support 3.9 (at least until October) # https://docs.python.org/3/library/typing.html#typing.Required # Is there a backport? bbox: list[float] - # Helpers MISSING = [None, ""] @@ -278,7 +293,7 @@ def __repr__(self): def signed_area( - coords: Coords, + coords: Points, fast: bool = False, ) -> float: """Return the signed area enclosed by a ring using the linear time @@ -296,7 +311,7 @@ def signed_area( return area2 / 2.0 -def is_cw(coords: Coords) -> bool: +def is_cw(coords: Points) -> bool: """Returns True if a polygon ring has clockwise orientation, determined by a negatively signed area. """ @@ -304,14 +319,14 @@ def is_cw(coords: Coords) -> bool: return area2 < 0 -def rewind(coords: Reversible[Coord]) -> Coords: +def rewind(coords: Reversible[Point]) -> Points: """Returns the input coords in reversed order.""" return list(reversed(coords)) -def ring_bbox(coords: Coords) -> BBox: +def ring_bbox(coords: Points) -> BBox: """Calculates and returns the bounding box of a ring.""" - xs, ys = zip(*coords) + xs, ys = map(list, list(zip(*coords))[:2]) # ignore any z or m values bbox = min(xs), min(ys), max(xs), max(ys) return bbox @@ -332,7 +347,7 @@ def bbox_contains(bbox1: BBox, bbox2: BBox) -> bool: return contains -def ring_contains_point(coords: Coords, p: Point2D) -> bool: +def ring_contains_point(coords: Points, p: Point2D) -> bool: """Fast point-in-polygon crossings algorithm, MacMartin optimization. Adapted from code by Eric Haynes @@ -381,7 +396,7 @@ class RingSamplingError(Exception): pass -def ring_sample(coords: Coords, ccw: bool = False) -> Point2D: +def ring_sample(coords: Points, ccw: bool = False) -> Point2D: """Return a sample point guaranteed to be within a ring, by efficiently finding the first centroid of a coordinate triplet whose orientation matches the orientation of the ring and passes the point-in-ring test. @@ -431,14 +446,15 @@ def itercoords(): ) -def ring_contains_ring(coords1: Coords, coords2: list[Point2D]) -> bool: +def ring_contains_ring(coords1: Points, coords2: list[Point]) -> bool: """Returns True if all vertexes in coords2 are fully inside coords1.""" - return all(ring_contains_point(coords1, p2) for p2 in coords2) + # Ignore Z and M values in coords2 + return all(ring_contains_point(coords1, p2[:2]) for p2 in coords2) def organize_polygon_rings( - rings: Iterable[Coords], return_errors: Optional[dict[str, int]] = None -) -> list[list[Coords]]: + rings: Iterable[Points], return_errors: Optional[dict[str, int]] = None +) -> list[list[Points]]: """Organize a list of coordinate rings into one or more polygons with holes. Returns a list of polygons, where each polygon is composed of a single exterior ring, and one or more interior holes. If a return_errors dict is provided (optional), @@ -992,8 +1008,8 @@ def __init__(self, shape: Optional[Shape] = None, record: Optional[_Record] = No def __geo_interface__(self) -> GeoJSONFeature: return { "type": "Feature", - "properties": None - if self.record is None + "properties": None + if self.record is None else self.record.as_dict(date_strings=True), "geometry": None if self.shape is None or self.shape.shapeType == NULL @@ -1015,11 +1031,8 @@ def __geo_interface__(self) -> GeoJSONGeometryCollection: # Note: currently this will fail if any of the shapes are null-geometries # could be fixed by storing the shapefile shapeType upon init, returning geojson type with empty coords collection = GeoJSONGeometryCollection( - type= "GeometryCollection", - geometries = [shape.__geo_interface__ - for shape in self - if shape is not None - ], + type="GeometryCollection", + geometries=[shape.__geo_interface__ for shape in self if shape is not None], ) return collection @@ -1035,10 +1048,6 @@ def __repr__(self): @property def __geo_interface__(self) -> GeoJSONFeatureCollection: - collection = { - "type": "FeatureCollection", - "features": [] #shaperec.__geo_interface__ for shaperec in self], - } return GeoJSONFeatureCollection( type="FeatureCollection", features=[shaperec.__geo_interface__ for shaperec in self], @@ -1362,7 +1371,7 @@ def __iter__(self): def __geo_interface__(self) -> GeoJSONFeatureCollectionWithBBox: shaperecords = self.shapeRecords() fcollection = GeoJSONFeatureCollectionWithBBox( - bbox = list(self.bbox), + bbox=list(self.bbox), **shaperecords.__geo_interface__, ) return fcollection