From f5ba1e1e1563bda530ebf8c039d6684f5bc28f43 Mon Sep 17 00:00:00 2001 From: sujata-m Date: Sat, 30 May 2026 20:21:18 +0530 Subject: [PATCH] Restrict null/NaN precheck to extension tags **PR Title** Restrict null/NaN precheck to extension tags ## Summary - Updated null/NaN precheck to inspect only top-level `ext:*` extension properties. - Left schema-owned tags to normal schema validation. - Continued rejecting actual JSON `null` and numeric `NaN` inside `ext:*`, including nested extension values. - Allowed string values like `"null"` and `"nan"` to pass the precheck. - Updated tests, README, changelog, and bumped package version to `0.4.2`. **Validation** - `.venv/bin/python -m unittest discover tests/unit_tests` passed: `166 tests` --- .github/workflows/unit_tests.yml | 35 +++++++++++++++++++ CHANGELOG.md | 3 ++ README.md | 12 +++---- src/python_osw_validation/__init__.py | 18 ++++++---- src/python_osw_validation/version.py | 2 +- tests/unit_tests/test_osw_validation.py | 8 ++--- .../unit_tests/test_osw_validation_extras.py | 30 ++++++++++++++-- 7 files changed, 89 insertions(+), 19 deletions(-) diff --git a/.github/workflows/unit_tests.yml b/.github/workflows/unit_tests.yml index 3ec38c3..1056508 100644 --- a/.github/workflows/unit_tests.yml +++ b/.github/workflows/unit_tests.yml @@ -1,9 +1,14 @@ --- name: Tests on: + push: + branches: [ develop, main ] pull_request: branches: [ develop, main ] +permissions: + contents: write + jobs: Tests: name: Unit tests @@ -55,6 +60,36 @@ jobs: - name: Check coverage run: | coverage report --fail-under=85 + + - name: Generate coverage badge + run: | + pip install coverage-badge + mkdir -p badge-out + coverage-badge -f -o badge-out/coverage.svg + + - name: Publish coverage badge to badges branch + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + set -e + cp badge-out/coverage.svg /tmp/coverage.svg + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + git fetch origin badges || true + if git show-ref --verify --quiet refs/remotes/origin/badges; then + git checkout badges + else + git checkout --orphan badges + git rm -rf . >/dev/null 2>&1 || true + fi + cp /tmp/coverage.svg coverage.svg + git add coverage.svg + if ! git diff --cached --quiet; then + git commit -m "chore: update coverage badge" + git push "https://x-access-token:${GITHUB_TOKEN}@github.com/${GITHUB_REPOSITORY}.git" HEAD:badges + else + echo "No badge changes to commit." + fi - name: Upload report to Azure uses: LanceMcCarthy/Action-AzureBlobUpload@v2 diff --git a/CHANGELOG.md b/CHANGELOG.md index 593278f..050c63b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ # Change log +### 0.4.2 - 2026-05-30 +- Changed the upfront null/NaN precheck to reject only actual JSON null values and numeric NaN values in `ext:*` extension properties; schema-owned properties are left to schema validation, and string values such as `"null"` and `"nan"` are no longer rejected by this precheck. + ### 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. diff --git a/README.md b/README.md index 3693246..f315bf0 100644 --- a/README.md +++ b/README.md @@ -1,8 +1,8 @@ # TDEI python lib OSW validation package -[![PyPI version](https://img.shields.io/pypi/v/python-osw-validation.svg)](https://pypi.org/project/python-osw-validation/) -[![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)](#) +[![python-osw-validation](https://img.shields.io/pypi/v/python-osw-validation?label=python-osw-validation&cacheSeconds=60&t=1)](https://pypi.org/project/python-osw-validation/) +[![Unit 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://raw.githubusercontent.com/TaskarCenterAtUW/TDEI-python-lib-osw-validation/actions/badges/coverage.svg) This package validates OSW GeoJSON datasets packaged as a ZIP file. @@ -17,7 +17,7 @@ This package validates OSW GeoJSON datasets packaged as a ZIP file. - Extracts the provided ZIP file - Finds supported OSW dataset files inside the extracted directory - Validates each file (`edges`, `lines`, `nodes`, `points`, `polygons`, and `zones`) against the matching schema -- Performs an upfront data-quality check for null-like placeholders in feature properties (for example `null`, `NaN`, `"null"`, `"nan"`) +- Performs an upfront data-quality check for actual null and numeric NaN values in `ext:*` extension properties (for example JSON `null` or numeric `NaN`; string values like `"null"` and `"nan"` are not rejected by this precheck) - Runs cross-file integrity checks such as duplicate `_id` detection and edge or zone references back to nodes - Returns a `ValidationResult` object with `is_valid`, `errors`, and `issues` @@ -50,8 +50,8 @@ print(result.issues) # capped by the same max_errors limit - `errors`: high-level validation messages, capped by `max_errors` (default `20`). - `issues`: detailed per-feature validation issues, also capped by `max_errors`. -- If null-like placeholders are found in feature `properties`, validation fails early before schema checks with actionable messages such as: - - `Invalid value at 'climb': 'null'. Null/NaN placeholders are not allowed; provide a valid value or remove this property.` +- If actual null or numeric NaN values are found in `ext:*` extension properties, validation fails early before schema checks with actionable messages such as: + - `Invalid value at 'ext:metadata.score': nan. Null/NaN placeholders are not allowed; provide a valid value or remove this property.` - For enum validation, long allowed-value lists are summarized as: - first 5 values joined by `|` - followed by `| and N more` when applicable. diff --git a/src/python_osw_validation/__init__.py b/src/python_osw_validation/__init__.py index 506cf51..127e851 100644 --- a/src/python_osw_validation/__init__.py +++ b/src/python_osw_validation/__init__.py @@ -2,6 +2,7 @@ import gc import json import math +import numbers from typing import Dict, Any, Optional, List, Tuple import geopandas as gpd import jsonschema_rs @@ -283,9 +284,7 @@ def _schema_key_from_text(self, text: Optional[str]) -> Optional[str]: def _is_nullish_value(self, value: Any) -> bool: if value is None: return True - if isinstance(value, str) and value.strip().lower() in {"null", "nan"}: - return True - return isinstance(value, float) and math.isnan(value) + return isinstance(value, numbers.Real) and math.isnan(value) def _collect_nullish_property_paths(self, obj: Any, prefix: str = "") -> List[Tuple[str, Any]]: paths: List[Tuple[str, Any]] = [] @@ -303,6 +302,13 @@ def _collect_nullish_property_paths(self, obj: Any, prefix: str = "") -> List[Tu paths.append((prefix or "value", obj)) return paths + def _collect_nullish_extension_property_paths(self, props: Dict[str, Any]) -> List[Tuple[str, Any]]: + paths: List[Tuple[str, Any]] = [] + for key, value in props.items(): + if isinstance(key, str) and key.startswith("ext:"): + paths.extend(self._collect_nullish_property_paths(value, key)) + return paths + def _contains_disallowed_features_for_02(self, geojson_data: Dict[str, Any]) -> set: """Detect Tree coverage or Custom content in legacy 0.2 datasets. @@ -676,8 +682,8 @@ def validate_osw_errors(self, file_path: str, max_errors: int) -> bool: filename = os.path.basename(file_path) - # Upfront guard: reject null/NaN values in feature properties. - # This runs before schema validation to surface data quality issues first. + # Upfront guard: reject null/NaN values in free-form extension properties. + # Schema-owned properties are left to schema validation. features = geojson_data.get("features", []) if isinstance(geojson_data, dict) else [] found_nullish = False for idx, feature in enumerate(features): @@ -686,7 +692,7 @@ def validate_osw_errors(self, file_path: str, max_errors: int) -> bool: props = feature.get("properties") if not isinstance(props, dict): continue - bad_paths = self._collect_nullish_property_paths(props) + bad_paths = self._collect_nullish_extension_property_paths(props) for path, bad_value in bad_paths: if len(self.errors) >= max_errors: return False diff --git a/src/python_osw_validation/version.py b/src/python_osw_validation/version.py index f0ede3d..a987347 100644 --- a/src/python_osw_validation/version.py +++ b/src/python_osw_validation/version.py @@ -1 +1 @@ -__version__ = '0.4.1' +__version__ = '0.4.2' diff --git a/tests/unit_tests/test_osw_validation.py b/tests/unit_tests/test_osw_validation.py index b1aff44..78aca91 100644 --- a/tests/unit_tests/test_osw_validation.py +++ b/tests/unit_tests/test_osw_validation.py @@ -281,18 +281,18 @@ def test_task_3469_issue_payload(self): self.assertIsInstance(result.issues, list) self.assertGreater(len(result.issues), 0) flattened = " | ".join(issue["error_message"][0] for issue in result.issues) - self.assertIn("Invalid value at 'climb': \"null\".", flattened) + self.assertIn("Invalid value at 'climb': 'null'.", flattened) - def test_issues_respect_max_errors_override(self): + def test_task_3469_string_nulls_go_through_schema_validation(self): validation = OSWValidation(zipfile_path=self.task_3469_file) default_result = validation.validate() self.assertFalse(default_result.is_valid) - self.assertEqual(len(default_result.issues), 20) + self.assertEqual(len(default_result.issues), 3) validation = OSWValidation(zipfile_path=self.task_3469_file) override_result = validation.validate(max_errors=500) self.assertFalse(override_result.is_valid) - self.assertGreater(len(override_result.issues), 20) + self.assertEqual(len(override_result.issues), 3) def test_issue_3297_issue_payload(self): validation = OSWValidation(zipfile_path=self.issue_3297_file) diff --git a/tests/unit_tests/test_osw_validation_extras.py b/tests/unit_tests/test_osw_validation_extras.py index 14b2961..f7f277d 100644 --- a/tests/unit_tests/test_osw_validation_extras.py +++ b/tests/unit_tests/test_osw_validation_extras.py @@ -102,6 +102,13 @@ def test_nullish_values_fail_before_schema_validation(self): "properties": { "step_count": None, "width": math.nan, + "string_null": "null", + "string_nan": "nan", + "string_nan_mixed_case": " NaN ", + "ext:missing": None, + "ext:metadata": { + "score": math.nan, + }, }, "geometry": {"type": "LineString", "coordinates": [[0, 0], [1, 1]]}, } @@ -117,11 +124,30 @@ def test_nullish_values_fail_before_schema_validation(self): self.assertEqual(len(validator.issues), 2) self.assertEqual( validator.issues[0]["error_message"], - ["Invalid value at 'step_count': None. Null/NaN placeholders are not allowed; provide a valid value or remove this property."], + ["Invalid value at 'ext:missing': None. Null/NaN placeholders are not allowed; provide a valid value or remove this property."], ) self.assertEqual( validator.issues[1]["error_message"], - ["Invalid value at 'width': nan. Null/NaN placeholders are not allowed; provide a valid value or remove this property."], + ["Invalid value at 'ext:metadata.score': nan. Null/NaN placeholders are not allowed; provide a valid value or remove this property."], + ) + + def test_non_extension_nullish_values_are_not_upfront_nullish_values(self): + validator = OSWValidation(zipfile_path="dummy.zip") + + self.assertEqual( + validator._collect_nullish_extension_property_paths({ + "step_count": None, + "width": math.nan, + "string_null": "null", + "string_nan": "nan", + "string_nan_mixed_case": " NaN ", + "nested": [None, math.nan], + "ext:string_null": "null", + "ext:string_nan": "nan", + "ext:string_nan_mixed_case": " NaN ", + "ext:nested_strings": ["null", "nan"], + }), + [], ) def test_missing_u_id_reports_error_without_keyerror(self):