Skip to content

Commit 45e70c0

Browse files
committed
Fix OSM→OSW zone _w_id references missing from nodes
## Dev Board Ticket - https://dev.azure.com/TDEI-UW/TDEI/_workitems/edit/3665 - https://dev.azure.com/TDEI-UW/TDEI/_workitems/edit/3191/ ## Changes - **Zone boundary protection in `construct_geometries()`**: collect zone boundary IDs from `_w_id` lists and filter them out of `internal_nodes` before `self.G.remove_nodes_from(...)`. This prevents zone boundary nodes from being deleted by simplification edge cases (e.g., circular-way neighbor combination), so they remain in `node_id_map` for `_w_id` → sequential `_id` remapping in `to_geojson()`. - **Regression fixture**: added [tests/unit_tests/test_files/zone_boundary.xml](tests/unit_tests/test_files/zone_boundary.xml) — a minimal `highway=pedestrian` plaza plus connecting footways with simplifiable intermediate nodes. - **Regression test**: `test_osm2osw_zone_boundary_is_osw_compliant` in [test_osm_compliance.py](tests/unit_tests/test_osm_compliance/test_osm_compliance.py) runs `Formatter.osm2osw()`, zips the output, and asserts `OSWValidation` reports zero issues. - **README fixes**: - Corrected invalid Python import `from osm-osw-reformatter` → `from osm_osw_reformatter`. - Aligned conda command (`python==3.10.3` → `python=3.10`) with the documented 3.10.x requirement. - Updated sample snippet to `return` the result and access `results.generated_files`. - **Version bump**: `0.3.4` → `0.3.5` in [version.py](src/osm_osw_reformatter/version.py) with matching [CHANGELOG.md](CHANGELOG.md) entry. ## Testing - Ran the full compliance test suite — all 3 pass, including the new zone-boundary regression test: ``` pytest tests/unit_tests/test_osm_compliance/ ===== 3 passed in 4.81s ===== ``` - Validated end-to-end via the `example.py` flow (`osm2osw` → zip → `python-osw-validation`) on the fixture: zero validation issues reported.
1 parent 0b4ead9 commit 45e70c0

9 files changed

Lines changed: 161 additions & 8 deletions

File tree

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
# Change log
22

3+
### 0.3.5
4+
- [BUG-3665](https://dev.azure.com/TDEI-UW/TDEI/_workitems/edit/3665) - Fix OSM→OSW export so zone boundary nodes are retained in `nodes.geojson` and zone `_w_id` references resolve to remapped sequential `_id`s, restoring OSW validation compliance for pedestrian-area geometries.
5+
- Add regression coverage that converts a `highway=pedestrian` plaza fixture and asserts `python-osw-validation` reports zero issues.
6+
- [ISSUE-3191](https://dev.azure.com/TDEI-UW/TDEI/_workitems/edit/3191/) - Fixed Documentation
7+
38
### 0.3.4
49
- Fix OSM→OSW conversion when OSM Way contains consecutive duplicate nodes (bug 3286). Instead of generating an invalid 0 length geometry, the segment is ignored.
510

README.md

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ The conversion of OSW data to OSM is beneficial for incorporating detailed pedes
3030
Running the code base requires a proper Python environment set up. The following lines of code helps one establish such env named `tdei-osw`. replace `tdei-osw` with the name of your choice.
3131

3232
```
33-
conda create -n tdei-osw python==3.10.3 gdal
33+
conda create -n tdei-osw python=3.10 gdal
3434
conda activate tdei-osw
3535
pip install -r requirements.txt
3636
```
@@ -92,24 +92,25 @@ To install the GDAL library (Geospatial Data Abstraction Library) on your system
9292
9393
```python
9494
import asyncio
95-
from osm-osw-reformatter import Formatter
95+
from osm_osw_reformatter import Formatter
9696
9797
async def osm_convert():
9898
f = Formatter(workdir=<OUTPUT_DIR>, file_path=<OSM_INPUT_FILE>)
99-
await f.osm2osw()
99+
return await f.osm2osw()
100100
# Uncomment below line to clean up the generated files
101101
# f.cleanup()
102102
103103
104104
def osw_convert():
105105
f = Formatter(workdir=<OUTPUT_DIR>, file_path=<OSW_INPUT_FILE>)
106-
f.osw2osm()
106+
return f.osw2osm()
107107
# Uncomment below line to clean up the generated files
108108
# f.cleanup()
109109
110110
111111
if __name__ == '__main__':
112-
asyncio.run(osm_convert())
112+
results = asyncio.run(osm_convert())
113+
print(results.generated_files)
113114
osw_convert()
114115
```
115116

requirements.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,4 +5,4 @@ shapely~=2.0.2
55
pyproj~=3.6.1
66
coverage~=7.5.1
77
ogr2osm==1.2.0
8-
python-osw-validation==0.3.5
8+
python-osw-validation==0.4.0

setup.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
'asyncio~=3.4.3',
3232
'networkx~=3.2',
3333
'shapely~=2.0.2',
34+
'pyproj~=3.6.1',
3435
'ogr2osm==1.2.0'
3536
],
3637
packages=find_packages(where='src'),

src/osm_osw_reformatter/serializer/osm/osm_graph.py

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -588,6 +588,21 @@ def construct_geometries(self, progressbar: Optional[callable] = None) -> None:
588588
if progressbar:
589589
progressbar.update(1)
590590

591+
# Protect zone boundary nodes: even if they ended up in internal_nodes
592+
# due to circular-way simplification edge cases, they must not be removed
593+
# because zones' _w_id still references them and to_geojson() needs to
594+
# remap those IDs to sequential _id values.
595+
zone_boundary_ids = set()
596+
for _n, _d in self.G.nodes(data=True):
597+
w_ids = _d.get("_w_id")
598+
if isinstance(w_ids, list):
599+
for ref in w_ids:
600+
try:
601+
zone_boundary_ids.add(int(ref))
602+
except (TypeError, ValueError):
603+
pass
604+
if zone_boundary_ids:
605+
internal_nodes = [n for n in internal_nodes if n not in zone_boundary_ids]
591606
self.G.remove_nodes_from(internal_nodes)
592607

593608
def to_undirected(self):
@@ -675,15 +690,30 @@ def _remap_node_ref(ref, node_id_map):
675690
zone_features = []
676691
polygon_features = []
677692
node_id_map = {}
693+
zone_node_refs = set()
694+
for _, d in self.G.nodes(data=True):
695+
if OSWZoneNormalizer.osw_zone_filter(d):
696+
w_ids = d.get("_w_id", d.get("ndref", []))
697+
if not isinstance(w_ids, list):
698+
w_ids = [w_ids]
699+
for ref in w_ids:
700+
zone_node_refs.add(ref)
701+
zone_node_refs.add(str(ref))
702+
try:
703+
zone_node_refs.add(int(ref))
704+
except (TypeError, ValueError):
705+
pass
706+
678707
for n, d in self.G.nodes(data=True):
679708
d_copy = {**d}
680709
source_id = _source_id(n)
681710
geometry_obj = d_copy.pop("geometry")
682711
geometry = mapping(geometry_obj)
683712
geometry_type = geometry_obj.geom_type
684713
is_topology_node = geometry_type == "Point" and self.G.degree(n) > 0
714+
is_zone_node = geometry_type == "Point" and n in zone_node_refs
685715

686-
if is_topology_node:
716+
if is_topology_node or is_zone_node:
687717
_assign_ids(d_copy, node_id_counter, source_id)
688718
node_id_map[n] = d_copy["_id"]
689719
node_id_counter += 1

src/osm_osw_reformatter/version.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
__version__ = '0.3.4'
1+
__version__ = '0.3.5'
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<osm version="0.6" generator="test" upload="false">
3+
<!-- Pedestrian plaza corners -->
4+
<node id="1" visible="true" version="1" lat="47.6151489" lon="-122.3206583"/>
5+
<node id="2" visible="true" version="1" lat="47.6149868" lon="-122.3206509"/>
6+
<node id="3" visible="true" version="1" lat="47.6149898" lon="-122.3203263"/>
7+
<node id="4" visible="true" version="1" lat="47.6151507" lon="-122.3203266"/>
8+
9+
<!-- External footway nodes (one intermediate node that should get simplified) -->
10+
<node id="5" visible="true" version="1" lat="47.6153000" lon="-122.3206583"/>
11+
<node id="6" visible="true" version="1" lat="47.6152500" lon="-122.3206583"/>
12+
<node id="7" visible="true" version="1" lat="47.6149898" lon="-122.3201000"/>
13+
<node id="8" visible="true" version="1" lat="47.6149898" lon="-122.3202000"/>
14+
15+
<!-- Pedestrian plaza: closed way forming a zone -->
16+
<way id="100" visible="true" version="1">
17+
<nd ref="1"/>
18+
<nd ref="2"/>
19+
<nd ref="3"/>
20+
<nd ref="4"/>
21+
<nd ref="1"/>
22+
<tag k="highway" v="pedestrian"/>
23+
<tag k="area" v="yes"/>
24+
<tag k="name" v="Test Plaza"/>
25+
</way>
26+
27+
<!-- Footway connecting external node 5 to plaza corner 1 via intermediate node 6 -->
28+
<way id="200" visible="true" version="1">
29+
<nd ref="5"/>
30+
<nd ref="6"/>
31+
<nd ref="1"/>
32+
<tag k="highway" v="footway"/>
33+
<tag k="footway" v="sidewalk"/>
34+
</way>
35+
36+
<!-- Footway connecting plaza corner 3 to external node 7 via intermediate node 8 -->
37+
<way id="201" visible="true" version="1">
38+
<nd ref="3"/>
39+
<nd ref="8"/>
40+
<nd ref="7"/>
41+
<tag k="highway" v="footway"/>
42+
<tag k="footway" v="sidewalk"/>
43+
</way>
44+
</osm>

tests/unit_tests/test_osm_compliance/test_osm_compliance.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
ROOT_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
1111
OUTPUT_DIR = os.path.join(os.path.dirname(os.path.dirname(ROOT_DIR)), 'output')
1212
TEST_DATA_WITH_INCLINE_ZIP_FILE = os.path.join(ROOT_DIR, 'test_files/dataset_with_incline.zip')
13+
TEST_ZONE_BOUNDARY_FILE = os.path.join(ROOT_DIR, 'test_files/zone_boundary.xml')
1314

1415

1516
class TestOSMCompliance(unittest.IsolatedAsyncioTestCase):
@@ -37,6 +38,25 @@ async def test_output_is_osm_compliant(self):
3738
os.remove(zip_path)
3839
formatter.cleanup()
3940

41+
async def test_osm2osw_zone_boundary_is_osw_compliant(self):
42+
formatter = Formatter(workdir=OUTPUT_DIR, file_path=TEST_ZONE_BOUNDARY_FILE, prefix='zone_boundary')
43+
res = await formatter.osm2osw()
44+
osw_files = res.generated_files
45+
46+
zip_path = os.path.join(OUTPUT_DIR, 'zone_boundary_osw.zip')
47+
with zipfile.ZipFile(zip_path, 'w', zipfile.ZIP_DEFLATED) as zipf:
48+
for f in osw_files:
49+
zipf.write(f, os.path.basename(f))
50+
51+
validator = OSWValidation(zipfile_path=zip_path)
52+
result = validator.validate()
53+
self.assertEqual(len(result.issues), 0, f'OSW Validation issues: {json.dumps(result.issues)}')
54+
55+
for f in osw_files:
56+
os.remove(f)
57+
os.remove(zip_path)
58+
formatter.cleanup()
59+
4060
async def test_incline_tag_preserved(self):
4161
osw2osm = OSW2OSM(
4262
zip_file_path=TEST_DATA_WITH_INCLINE_ZIP_FILE,

tests/unit_tests/test_serializer/test_osm_graph.py

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1058,6 +1058,58 @@ def test_to_geojson_zones_reference_remapped_node_ids_in_w_id(self):
10581058
zone_w_ids = zone_data["features"][0]["properties"]["_w_id"]
10591059
self.assertTrue(all(wid in node_ids for wid in zone_w_ids))
10601060

1061+
def test_to_geojson_zone_boundary_custom_points_stay_in_nodes_file(self):
1062+
graph = nx.MultiDiGraph()
1063+
graph.add_node(
1064+
10,
1065+
geometry=Point(0, 0),
1066+
lon=0.0,
1067+
lat=0.0,
1068+
**{"ext:entrance": "yes"},
1069+
)
1070+
graph.add_node(20, geometry=Point(1, 0), lon=1.0, lat=0.0)
1071+
graph.add_node(
1072+
"z100",
1073+
geometry=Polygon([(0, 0), (1, 0), (0, 1), (0, 0)]),
1074+
highway="pedestrian",
1075+
_w_id=["10", "20"],
1076+
)
1077+
1078+
osm_graph = OSMGraph(G=graph)
1079+
1080+
with TemporaryDirectory() as tmpdir:
1081+
nodes_path = os.path.join(tmpdir, 'nodes.geojson')
1082+
edges_path = os.path.join(tmpdir, 'edges.geojson')
1083+
points_path = os.path.join(tmpdir, 'points.geojson')
1084+
lines_path = os.path.join(tmpdir, 'lines.geojson')
1085+
zones_path = os.path.join(tmpdir, 'zones.geojson')
1086+
polygons_path = os.path.join(tmpdir, 'polygons.geojson')
1087+
1088+
osm_graph.to_geojson(
1089+
nodes_path,
1090+
edges_path,
1091+
points_path,
1092+
lines_path,
1093+
zones_path,
1094+
polygons_path,
1095+
)
1096+
1097+
with open(nodes_path) as f:
1098+
node_data = json.load(f)
1099+
with open(zones_path) as f:
1100+
zone_data = json.load(f)
1101+
1102+
node_ids = {feat["properties"]["_id"] for feat in node_data["features"]}
1103+
node_ext_ids = {
1104+
feat["properties"].get("ext:osm_id")
1105+
for feat in node_data["features"]
1106+
}
1107+
zone_w_ids = zone_data["features"][0]["properties"]["_w_id"]
1108+
1109+
self.assertIn("10", node_ext_ids)
1110+
self.assertFalse(os.path.exists(points_path))
1111+
self.assertTrue(all(wid in node_ids for wid in zone_w_ids))
1112+
10611113
def test_to_undirected_on_simple_graph(self):
10621114
g = nx.Graph()
10631115
g.add_edge(1, 2)

0 commit comments

Comments
 (0)