Merge pull request #32 from TaskarCenterAtUW/feature-1679

Fixed 1679

Author: sujata-m

CHANGELOG.md

@@ -1,5 +1,10 @@
 # Change log
 
+### 0.2.10
+
+- Added limit the message error when u_id and v_id are missing
+- Added Unit test cases for missing u_id and v_id
+
 ### 0.2.8
 
 - Fixed geopands version to `0.14.4`.

src/example.py

@@ -4,7 +4,7 @@
 PARENT_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
 ASSETS_DIR = os.path.join(PARENT_DIR, 'tests/assets')
 VALID_ZIP_FILE = os.path.join(ASSETS_DIR, 'valid.zip')
-INVALID_ZIP_FILE = os.path.join(ASSETS_DIR, 'invalid.zip')
+INVALID_ZIP_FILE = os.path.join(ASSETS_DIR, '4151.zip')
 INVALID_VANCOUVER_ZIP_FILE = os.path.join(ASSETS_DIR, 'vancouver-dataset.zip')
 SCHEMA_DIR = os.path.join(PARENT_DIR, 'src/python_osw_validation/schema')
 SCHEMA_FILE_PATH = os.path.join(SCHEMA_DIR, 'opensidewalks.schema.json')
@@ -33,6 +33,7 @@ def invalid_test_without_provided_schema():
     validator = OSWValidation(zipfile_path=INVALID_ZIP_FILE)
     result = validator.validate(max_errors=10)
     print(f'Number of errors: {len(result.errors)}')
+    print(result.errors)
     print(f'Invalid Test With Provided Schema: {"Failed" if result.is_valid else "Passed"}')
 
 

src/python_osw_validation/__init__.py

@@ -107,22 +107,40 @@ def validate(self, max_errors=20) -> ValidationResult:
             unmatched = node_ids_edges_u - node_ids
             is_valid = len(unmatched) == 0
             if not is_valid:
+                unmatched_list = list(unmatched)
+                num_unmatched = len(unmatched_list)
+                limit = min(num_unmatched, 20)
+                displayed_unmatched = ', '.join(map(str, unmatched_list[:limit]))
                 self.errors.append(
-                    f"All _u_id's in edges should be part of _id's mentioned in nodes, _u_id's not in nodes are: {unmatched}")
+                    f"All _u_id's in edges should be part of _id's mentioned in nodes. "
+                    f"Showing {'20' if num_unmatched > 20 else 'all'} out of {len(unmatched)} unmatched _u_id's: {displayed_unmatched}"
+                )
 
             # Do all node references in _v_id exist in nodes?
             unmatched = node_ids_edges_v - node_ids
             is_valid = len(unmatched) == 0
             if not is_valid:
+                unmatched_list = list(unmatched)
+                num_unmatched = len(unmatched_list)
+                limit = min(num_unmatched, 20)
+                displayed_unmatched = ', '.join(map(str, unmatched_list[:limit]))
                 self.errors.append(
-                    f"All _v_id's in edges should be part of _id's mentioned in nodes, _v_id's not in nodes are: {unmatched}")
+                    f"All _v_id's in edges should be part of _id's mentioned in nodes. "
+                    f"Showing {'20' if num_unmatched > 20 else 'all'} out of {len(unmatched)} unmatched _v_id's: {displayed_unmatched}"
+                )
 
             # Do all node references in _w_id exist in nodes?
             unmatched = node_ids_zones_w - node_ids
             is_valid = len(unmatched) == 0
             if not is_valid:
+                unmatched_list = list(unmatched)
+                num_unmatched = len(unmatched_list)
+                limit = min(num_unmatched, 20)
+                displayed_unmatched = ', '.join(map(str, unmatched_list[:limit]))
                 self.errors.append(
-                    f"All _w_id's in zones should be part of _id's mentioned in nodes, _w_id's not in nodes are: {unmatched}")
+                    f"All _w_id's in zones should be part of _id's mentioned in nodes. "
+                    f"Showing {'20' if num_unmatched > 20 else 'all'} out of {len(unmatched)} unmatched _w_id's: {displayed_unmatched}"
+                )
 
             # Geometry validation: check geometry type in each file and test if coordinates make a shape that is reasonable geometric shape according to the Simple Feature Access standard
             for osw_file in OSW_DATASET:
@@ -131,8 +149,14 @@ def validate(self, max_errors=20) -> ValidationResult:
                                 OSW_DATASET[osw_file].is_valid == False)]
                 is_valid = len(invalid_geojson) == 0
                 if not is_valid:
+                    invalid_ids = list(set(invalid_geojson['_id']))
+                    num_invalid = len(invalid_ids)
+                    limit = min(num_invalid, 20)
+                    displayed_invalid = ', '.join(map(str, invalid_ids[:min(num_invalid, limit)]))
                     self.errors.append(
-                        f"Invalid {osw_file} geometries found, id's of invalid geometries: {set(invalid_geojson['_id'])}")
+                        f"Showing {'20' if num_invalid > 20 else 'all'} out of {num_invalid} invalid {osw_file} geometries, "
+                        f"id's of invalid geometries: {displayed_invalid}"
+                        )
 
             # Validate OSW external extensions
             for file in validator.externalExtensions:

src/python_osw_validation/version.py

@@ -1 +1 @@
-__version__ = '0.2.9'
+__version__ = '0.2.10'

tests/assets/4151.zip

@@ -29,6 +29,7 @@ def setUp(self):
         self.valid_zones_file = os.path.join(ASSETS_PATH, 'UW.zones.valid.zip')
         self.invalid_zones_file = os.path.join(ASSETS_PATH, 'UW.zones.invalid.zip')
         self.valid_osw_file = os.path.join(ASSETS_PATH, 'wa.bellevue.zip')
+        self.invalid_v_id_file = os.path.join(ASSETS_PATH, '4151.zip')
         self.schema_file_path = SCHEMA_FILE_PATH
         self.invalid_schema_file_path = INVALID_SCHEMA_FILE_PATH
 
@@ -45,7 +46,6 @@ def test_valid_zipfile_with_schema(self):
         self.assertIsNone(result.errors)
 
     def test_valid_zipfile_with_invalid_schema(self):
-
         validation = OSWValidation(zipfile_path=self.valid_zipfile, schema_file_path=self.invalid_schema_file_path)
         result = validation.validate()
         self.assertTrue(len(result.errors) > 0)
@@ -95,7 +95,7 @@ def test_invalid_zipfile_should_specific_errors_counts(self):
 
     def test_invalid_zipfile_with_invalid_schema(self):
         validation = OSWValidation(zipfile_path=self.invalid_zipfile,
-                                       schema_file_path=self.invalid_schema_file_path)
+                                   schema_file_path=self.invalid_schema_file_path)
         result = validation.validate()
         self.assertTrue(len(result.errors) > 0)
 
@@ -160,7 +160,8 @@ def test_external_extension_file_inside_zipfile(self):
         self.assertIsNone(result.errors)
 
     def test_external_extension_file_inside_zipfile_with_schema(self):
-        validation = OSWValidation(zipfile_path=self.external_extension_file_zipfile, schema_file_path=self.schema_file_path)
+        validation = OSWValidation(zipfile_path=self.external_extension_file_zipfile,
+                                   schema_file_path=self.schema_file_path)
         result = validation.validate()
         self.assertTrue(result.is_valid)
         self.assertIsNone(result.errors)
@@ -226,6 +227,29 @@ def test_invalid_zones_file(self):
         self.assertFalse(result.is_valid)
         self.assertIsNotNone(result.errors)
 
+    def test_unmatched_ids_limited_to_20(self):
+        validation = OSWValidation(zipfile_path=self.invalid_v_id_file)
+        result = validation.validate()
+
+        # Ensure validation fails
+        self.assertFalse(result.is_valid, 'Validation should fail, but it passed.')
+        self.assertTrue(result.errors, 'Validation should produce errors, but it returned none.')
+
+        # Try to find the unmatched ID error message
+        error_message = next((err for err in result.errors if 'unmatched' in err.lower()), None)
+
+        # Ensure the error message exists
+        self.assertIsNotNone(error_message, 'Expected error message for unmatched IDs not found.')
+
+        # Extract the displayed IDs from the message
+        extracted_ids = error_message.split(':')[-1].strip().split(', ')
+
+        # Ensure only 20 IDs are displayed
+        self.assertLessEqual(len(extracted_ids), 20, 'More than 20 unmatched IDs displayed in the error message.')
+
+        # Ensure the total count is mentioned
+        self.assertIn('Showing 20 out of', error_message)
+
 
 if __name__ == '__main__':
     unittest.main()