diff --git a/.coveragerc b/.coveragerc new file mode 100644 index 0000000..60bdfcb --- /dev/null +++ b/.coveragerc @@ -0,0 +1,4 @@ +[run] +omit = + src/python_osw_validation/version.py + src/python_osw_validation/example.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 265d434..593278f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Change log +### 0.4.1 - 2026-05-28 +- Fixed misleading `Expecting value: line 1 column 1 (char 0)` failure when loading GeoJSON files containing `ext:*` extension properties with mixed value types across features (e.g. `ext:TextRotation` numeric in some features and string in others). Extension properties are now stripped before the internal GeoPandas load used for geometry and `_id` integrity checks; schema-level validation of `ext:*` properties is unchanged. +- Added regression coverage for the mixed-type `ext:*` load path in `tests/unit_tests/test_helpers.py` and added `tests/assets/SDOT_lanewidth_osw.points.geojson.zip` as the reproducer dataset. +- Excluded `version.py` from the coverage report via a new `.coveragerc`. + ### 0.4.0 - 2026-04-30 - Added geometry mapping consistency validation: `_u_id` and `_v_id` references in edges are now verified against the actual start/end coordinates of the referenced node geometries. - Added `_w_id` coordinate mapping validation for zones: each referenced node coordinate must be a vertex of the zone's polygon exterior ring, in order. diff --git a/README.md b/README.md index ba98dd9..547327a 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,8 @@ # TDEI python lib OSW validation package +[![Tests](https://github.com/TaskarCenterAtUW/TDEI-python-lib-osw-validation/actions/workflows/unit_tests.yml/badge.svg)](https://github.com/TaskarCenterAtUW/TDEI-python-lib-osw-validation/actions/workflows/unit_tests.yml) +[![Coverage](https://img.shields.io/badge/coverage-92%25-brightgreen)](#) + This package validates OSW GeoJSON datasets packaged as a ZIP file. ## System requirements diff --git a/src/python_osw_validation/__init__.py b/src/python_osw_validation/__init__.py index cac2a70..506cf51 100644 --- a/src/python_osw_validation/__init__.py +++ b/src/python_osw_validation/__init__.py @@ -14,6 +14,7 @@ _err_kind, _feature_index_from_error, _pretty_message, + _read_geojson_without_ext, ) SCHEMA_PATH = os.path.join(os.path.dirname(__file__), 'schema') @@ -419,7 +420,7 @@ def _finalize(is_valid: bool, errors: Optional[List[str]] = None) -> ValidationR osw_file = next((osw_key for osw_key in OSW_DATASET_FILES.keys() if osw_key in os.path.basename(file_path)), '') try: - gdf = gpd.read_file(file_path) + gdf = _read_geojson_without_ext(file_path) except Exception as e: self.log_errors( message=f"Failed to read '{os.path.basename(file_path)}' as GeoJSON: {e}", @@ -557,7 +558,7 @@ def _finalize(is_valid: bool, errors: Optional[List[str]] = None) -> ValidationR file_path = os.path.join(file) file_name = os.path.basename(file) try: - extensionFile = gpd.read_file(file_path) + extensionFile = _read_geojson_without_ext(file_path) except Exception as e: self.log_errors( message=f"Failed to read extension '{file_name}' as GeoJSON: {e}", diff --git a/src/python_osw_validation/helpers.py b/src/python_osw_validation/helpers.py index 4f971b0..fc528f9 100644 --- a/src/python_osw_validation/helpers.py +++ b/src/python_osw_validation/helpers.py @@ -1,6 +1,30 @@ from typing import Optional +import json import re +import geopandas as gpd + + +def _read_geojson_without_ext(file_path: str) -> gpd.GeoDataFrame: + """Load a GeoJSON file into a GeoDataFrame with ext:* properties removed. + + Why: pyogrio/GDAL infers a single dtype per property column when reading + GeoJSON. Extension tags (ext:*) are free-form by spec and may legitimately + mix numbers and strings across features, which breaks that inference and + surfaces as a confusing JSON parse error. Schema validation has already + accepted these properties; the GeoDataFrame is only used for geometry and + _id-based integrity checks, so dropping ext:* here is safe. + """ + with open(file_path, 'r') as f: + data = json.load(f) + for feature in data.get('features', []): + props = feature.get('properties') + if isinstance(props, dict): + for key in [k for k in props if isinstance(k, str) and k.startswith('ext:')]: + del props[key] + crs = (data.get('crs') or {}).get('properties', {}).get('name') + return gpd.GeoDataFrame.from_features(data.get('features', []), crs=crs) + _ADDITIONAL_PROPERTIES_RE = re.compile( r"Additional properties are not allowed \('(?P[^']+)' was unexpected\)" ) diff --git a/src/python_osw_validation/version.py b/src/python_osw_validation/version.py index abeeedb..f0ede3d 100644 --- a/src/python_osw_validation/version.py +++ b/src/python_osw_validation/version.py @@ -1 +1 @@ -__version__ = '0.4.0' +__version__ = '0.4.1' diff --git a/tests/assets/SDOT_lanewidth_osw.points.geojson.zip b/tests/assets/SDOT_lanewidth_osw.points.geojson.zip new file mode 100644 index 0000000..1721dd4 Binary files /dev/null and b/tests/assets/SDOT_lanewidth_osw.points.geojson.zip differ diff --git a/tests/unit_tests/test_helpers.py b/tests/unit_tests/test_helpers.py index 9e604b4..641b4de 100644 --- a/tests/unit_tests/test_helpers.py +++ b/tests/unit_tests/test_helpers.py @@ -1,4 +1,8 @@ +import json +import os +import tempfile import unittest + import src.python_osw_validation.helpers as helpers @@ -179,5 +183,97 @@ def test_tiebreaker_shorter_message_is_better(self): self.assertLess(helpers._rank_for(e_short), helpers._rank_for(e_long)) +class TestReadGeojsonWithoutExt(unittest.TestCase): + def _write_geojson(self, payload): + fd, path = tempfile.mkstemp(suffix=".geojson") + os.close(fd) + with open(path, "w") as f: + json.dump(payload, f) + self.addCleanup(os.remove, path) + return path + + def test_drops_ext_properties_with_mixed_types(self): + """The bug we're fixing: ext:* values mixing numeric and string across + features causes pyogrio dtype inference to fail. After stripping, the + load should succeed and the ext:* column should not be present.""" + payload = { + "type": "FeatureCollection", + "features": [ + { + "type": "Feature", + "properties": {"_id": "a", "ext:TextRotation": 310.0, "name": "n1"}, + "geometry": {"type": "Point", "coordinates": [0, 0]}, + }, + { + "type": "Feature", + "properties": {"_id": "b", "ext:TextRotation": "not_appl", "name": "n2"}, + "geometry": {"type": "Point", "coordinates": [1, 1]}, + }, + ], + } + path = self._write_geojson(payload) + gdf = helpers._read_geojson_without_ext(path) + + self.assertEqual(len(gdf), 2) + self.assertNotIn("ext:TextRotation", gdf.columns) + # Non-ext columns must be preserved for downstream integrity checks. + self.assertIn("_id", gdf.columns) + self.assertIn("name", gdf.columns) + self.assertEqual(sorted(gdf["_id"].tolist()), ["a", "b"]) + + def test_preserves_non_ext_properties(self): + payload = { + "type": "FeatureCollection", + "features": [ + { + "type": "Feature", + "properties": {"_id": "x", "highway": "footway", "ext:foo": 1}, + "geometry": {"type": "Point", "coordinates": [0, 0]}, + } + ], + } + gdf = helpers._read_geojson_without_ext(self._write_geojson(payload)) + self.assertIn("highway", gdf.columns) + self.assertNotIn("ext:foo", gdf.columns) + self.assertEqual(gdf["highway"].iloc[0], "footway") + + def test_no_ext_properties_is_noop(self): + payload = { + "type": "FeatureCollection", + "features": [ + { + "type": "Feature", + "properties": {"_id": "x", "highway": "footway"}, + "geometry": {"type": "Point", "coordinates": [2, 3]}, + } + ], + } + gdf = helpers._read_geojson_without_ext(self._write_geojson(payload)) + self.assertEqual(len(gdf), 1) + self.assertEqual(gdf["highway"].iloc[0], "footway") + self.assertEqual(gdf.geometry.iloc[0].x, 2) + self.assertEqual(gdf.geometry.iloc[0].y, 3) + + def test_empty_feature_collection(self): + payload = {"type": "FeatureCollection", "features": []} + gdf = helpers._read_geojson_without_ext(self._write_geojson(payload)) + self.assertEqual(len(gdf), 0) + + def test_crs_is_propagated_when_present(self): + payload = { + "type": "FeatureCollection", + "crs": {"type": "name", "properties": {"name": "EPSG:4326"}}, + "features": [ + { + "type": "Feature", + "properties": {"_id": "x"}, + "geometry": {"type": "Point", "coordinates": [0, 0]}, + } + ], + } + gdf = helpers._read_geojson_without_ext(self._write_geojson(payload)) + self.assertIsNotNone(gdf.crs) + + if __name__ == "__main__": unittest.main() diff --git a/tests/unit_tests/test_osw_validation_extras.py b/tests/unit_tests/test_osw_validation_extras.py index b136bb3..14b2961 100644 --- a/tests/unit_tests/test_osw_validation_extras.py +++ b/tests/unit_tests/test_osw_validation_extras.py @@ -16,7 +16,7 @@ _PATCH_UNIQUE = f"{_PATCH_PREFIX}.OSWValidation.are_ids_unique" _PATCH_ZIP = f"{_PATCH_PREFIX}.ZipFileHandler" _PATCH_EV = f"{_PATCH_PREFIX}.ExtractedDataValidator" -_PATCH_READ_FILE = f"{_PATCH_PREFIX}.gpd.read_file" +_PATCH_READ_FILE = f"{_PATCH_PREFIX}._read_geojson_without_ext" _PATCH_VALIDATE = f"{_PATCH_PREFIX}.OSWValidation.validate_osw_errors" _PATCH_DATASET_FILES = f"{_PATCH_PREFIX}.OSW_DATASET_FILES"