Merge pull request #22 from TaskarCenterAtUW/bugfix-1654

Bugfix 1654

Author: sujata-m

CHANGELOG.md

@@ -1,5 +1,15 @@
 # Change log
 
+### 0.2.7
+- Fixed [BUG 1654](https://dev.azure.com/TDEI-UW/TDEI/_workitems/edit/1654)
+- Added functionality to retain the `ext` tags
+## Unit Tests
+- Verified all output files are valid GeoJSON FeatureCollections
+- Ensured `nodes` files contain only Point geometries
+- Validated that all feature `_id` properties are strings
+- Asserted no features are missing geometry or coordinates
+- Checked that no duplicate `_id` values exist within any generated file
+
 ### 0.2.6
 - Added unit test cases
 - Added unit test cases pipeline

src/osm_osw_reformatter/serializer/osm/osm_graph.py

@@ -162,6 +162,9 @@ def way(self, w):
         for i in range(len(w.nodes)):
             u = w.nodes[i]
 
+            if not u.location.valid():
+                continue
+
             u_lon = float(u.lon)
             u_lat = float(u.lat)
 
@@ -304,6 +307,17 @@ def area(self, a):
                 self.G.add_node("g" + str(a.id), **d3)
             exteriors_count = exteriors_count + 1
 
+class OSMTaggedNodeParser(osmium.SimpleHandler):
+    def __init__(self, G: nx.MultiDiGraph):
+        osmium.SimpleHandler.__init__(self)
+        self.G = G
+
+    def node(self, n):
+        # Only add nodes with tags
+        if n.tags and len(n.tags) > 0:
+            d = dict(n.tags)
+            # Store OSM node id as string (to match the pattern in your output)
+            self.G.add_node(n.id, lon=n.location.lon, lat=n.location.lat, **d)
 
 class OSMGraph:
     def __init__(self, G: nx.MultiDiGraph = None) -> None:
@@ -313,6 +327,11 @@ def __init__(self, G: nx.MultiDiGraph = None) -> None:
         # Geodesic distance calculator. Assumes WGS84-like geometries.
         self.geod = pyproj.Geod(ellps='WGS84')
 
+    def node(self, n):
+        if len(n.tags) > 0 and n.id not in self.G.nodes:
+            d = dict(n.tags)
+            self.G.add_node(n.id, lon=n.location.lon, lat=n.location.lat, **d)
+
     @classmethod
     def from_osm_file(
       self, osm_file, way_filter: Optional[callable] = None, node_filter: Optional[callable] = None,
@@ -339,6 +358,13 @@ def from_osm_file(
         G = line_parser.G
         del line_parser
 
+        # --- PATCH START: Add all loose/tagged nodes ---
+        tagged_node_parser = OSMTaggedNodeParser(G)
+        tagged_node_parser.apply_file(osm_file)
+        G = tagged_node_parser.G
+        del tagged_node_parser
+        # --- PATCH END ---
+
         # zone_parser = OSMZoneParser(G, zone_filter, progressbar=progressbar)
         # zone_parser.apply_file(osm_file)
         # G = zone_parser.G
@@ -570,6 +596,8 @@ def to_geojson(self, *args) -> None:
             d_copy['_u_id'] = str(u)
             d_copy['_v_id'] = str(v)
 
+            d_copy['ext:osm_id'] = str(d['osm_id'])
+
             if 'osm_id' in d_copy:
                 d_copy.pop('osm_id')
 
@@ -591,6 +619,7 @@ def to_geojson(self, *args) -> None:
         for n, d in self.G.nodes(data=True):
             d_copy = {**d}
             d_copy["_id"] = str(n)[1:]
+            d_copy['ext:osm_id'] = str(d_copy.get('osm_id', d_copy["_id"]))
 
             if OSWPointNormalizer.osw_point_filter(d):
                 geometry = mapping(d_copy.pop("geometry"))

src/osm_osw_reformatter/serializer/osm/osm_normalizer.py

@@ -38,5 +38,12 @@ def process_feature_post(self, osmgeometry, ogrfeature, ogrgeometry):
         ogr feature and ogr geometry used to create the object are passed as
         well. Note that any return values will be discarded by ogr2osm.
         '''
-        if osmgeometry.tags['_id'][0]:
-            osmgeometry.id = int(osmgeometry.tags.pop('_id')[0])
+        osm_id = None
+        # ext:osm_id is probably in the tags dictionary as 'ext:osm_id' or similar
+        if 'ext:osm_id' in osmgeometry.tags and osmgeometry.tags['ext:osm_id'][0]:
+            osm_id = int(osmgeometry.tags['ext:osm_id'][0])
+        elif '_id' in osmgeometry.tags and osmgeometry.tags['_id'][0]:
+            osm_id = int(osmgeometry.tags['_id'][0])
+
+        if osm_id is not None:
+            osmgeometry.id = osm_id

src/osm_osw_reformatter/serializer/osw/osw_normalizer.py

@@ -249,7 +249,8 @@ def normalize(self):
         elif self.is_street_lamp():
             return self._normalize_point({"highway": str})
         else:
-            raise ValueError("This is an invalid point")
+            print(f"Invalid point skipped. Tags: {self.tags}")
+            return {}
 
     def _normalize_point(self, keep_keys={}, defaults = {}):
         generic_keep_keys = {}
@@ -503,7 +504,13 @@ def _normalize(tags, keep_keys, defaults):
             pass
 
     # Preserve order of keep_keys first followed by defaults
-    return {**{**new_tags, **defaults}, **new_tags}
+    new_tags.update(defaults)
+
+    # Keep all tags that start with "ext:"
+    ext_tags = {k: v for k, v in tags.items() if k.startswith("ext:")}
+
+    return {**{**new_tags, **defaults}, **{**new_tags, **ext_tags}}
+
 
 def tactile_paving(tag_value, tags):
     if tag_value.lower() not in (

src/osm_osw_reformatter/version.py

@@ -1 +1 @@
-__version__ = '0.2.6'
+__version__ = '0.2.7'

tests/unit_tests/test_files/test_roundtrip.xml

@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<osm version="0.6" generator="ogr2osm 1.2.0" upload="false">
+<node visible="true" id="22592328" lat="34.052143" lon="-118.2672968"><tag k="_id" v="22592328"/></node>
+<node visible="true" id="726963844" lat="34.0517032" lon="-118.2675859"><tag k="_id" v="726963844"/></node>
+<node visible="true" id="20393501" lat="34.0521803" lon="-118.2673793"><tag k="_id" v="20393501"/></node>
+<way visible="true" id="1"><nd ref="22592328"/><nd ref="726963844"/><tag k="highway" v="residential"/><tag k="name" v="Hartford Avenue"/><tag k="ext:qm:fixed" v="33"/><tag k="ext:osm_id" v="1"/><tag k="_id" v="1"/><tag k="ext:my_surface" v="asphalt"/><tag k="ext:my_color" v="green"/></way>
+<way visible="true" id="2"><nd ref="22592328"/><nd ref="20393501"/><tag k="highway" v="tertiary"/><tag k="surface" v="asphalt"/><tag k="name" v="West 7th Street"/><tag k="ext:qm:fixed" v="38"/><tag k="ext:osm_id" v="2"/><tag k="_id" v="2"/><tag k="ext:my_surface" v="asphalt"/><tag k="ext:my_color" v="red"/></way>
+</osm>

tests/unit_tests/test_files/test_roundtrip.zip

@@ -1,5 +1,6 @@
 import os
 import re
+import json
 import asyncio
 import unittest
 from src.osm_osw_reformatter.osm2osw.osm2osw import OSM2OSW
@@ -69,6 +70,120 @@ async def mock_count_entities_error(osm_file_path, counter_cls):
         result = await osm2osw.convert()
         self.assertFalse(result.status)
 
+    def test_ext_tags_present_in_output(self):
+        osm_file_path = TEST_FILE
+
+        async def run_test():
+            osm2osw = OSM2OSW(osm_file=osm_file_path, workdir=OUTPUT_DIR, prefix='test')
+            result = await osm2osw.convert()
+            self.assertTrue(result.status)
+
+            has_ext_tag = False
+            for file_path in result.generated_files:
+                if file_path.endswith('.geojson'):
+                    with open(file_path) as f:
+                        geojson = json.load(f)
+                        for feature in geojson.get('features', []):
+                            props = feature.get('properties', {})
+                            if any(k.startswith("ext:") for k in props):
+                                has_ext_tag = True
+                                break
+                    if has_ext_tag:
+                        break
+
+            self.assertTrue(has_ext_tag, "No ext: tags found in generated GeoJSON features")
+
+            for file_path in result.generated_files:
+                os.remove(file_path)
+
+        asyncio.run(run_test())
+
+    def test_nodes_file_has_point_geometry(self):
+        osm_file_path = TEST_FILE
+
+        async def run_test():
+            osm2osw = OSM2OSW(osm_file=osm_file_path, workdir=OUTPUT_DIR, prefix='test')
+            result = await osm2osw.convert()
+            self.assertTrue(result.status)
+
+            for file_path in result.generated_files:
+                if "nodes" in file_path:
+                    with open(file_path) as f:
+                        geojson = json.load(f)
+                        for feature in geojson["features"]:
+                            self.assertEqual(feature["geometry"]["type"], "Point")
+                    break
+
+            for file_path in result.generated_files:
+                os.remove(file_path)
+
+        asyncio.run(run_test())
+
+    def test_all_feature_ids_are_strings(self):
+        osm_file_path = TEST_FILE
+
+        async def run_test():
+            osm2osw = OSM2OSW(osm_file=osm_file_path, workdir=OUTPUT_DIR, prefix='test')
+            result = await osm2osw.convert()
+            self.assertTrue(result.status)
+
+            for file_path in result.generated_files:
+                with open(file_path) as f:
+                    geojson = json.load(f)
+                    for feature in geojson.get("features", []):
+                        self.assertIn("_id", feature["properties"])
+                        self.assertIsInstance(feature["properties"]["_id"], str)
+
+            for file_path in result.generated_files:
+                os.remove(file_path)
+
+        asyncio.run(run_test())
+
+
+    def test_no_empty_features(self):
+        osm_file_path = TEST_FILE
+
+        async def run_test():
+            osm2osw = OSM2OSW(osm_file=osm_file_path, workdir=OUTPUT_DIR, prefix='test')
+            result = await osm2osw.convert()
+            self.assertTrue(result.status)
+
+            for file_path in result.generated_files:
+                with open(file_path) as f:
+                    geojson = json.load(f)
+                    for feature in geojson.get("features", []):
+                        self.assertIn("geometry", feature)
+                        self.assertIsNotNone(feature["geometry"])
+                        self.assertIn("type", feature["geometry"])
+                        self.assertIn("coordinates", feature["geometry"])
+
+            for file_path in result.generated_files:
+                os.remove(file_path)
+
+        asyncio.run(run_test())
+
+    def test_no_duplicate_ids_in_file(self):
+        osm_file_path = TEST_FILE
+
+        async def run_test():
+            osm2osw = OSM2OSW(osm_file=osm_file_path, workdir=OUTPUT_DIR, prefix='test')
+            result = await osm2osw.convert()
+            self.assertTrue(result.status)
+
+            for file_path in result.generated_files:
+                with open(file_path) as f:
+                    geojson = json.load(f)
+                    seen_ids = set()
+                    for feature in geojson.get("features", []):
+                        _id = feature["properties"].get("_id")
+                        self.assertNotIn(_id, seen_ids, f"Duplicate _id: {_id} in {file_path}")
+                        seen_ids.add(_id)
+
+            for file_path in result.generated_files:
+                os.remove(file_path)
+
+        asyncio.run(run_test())
+
 
 if __name__ == '__main__':
     unittest.main()