diff --git a/city2graph/morphology.py b/city2graph/morphology.py index d01b05d..0cc6b33 100644 --- a/city2graph/morphology.py +++ b/city2graph/morphology.py @@ -2338,9 +2338,17 @@ def _add_building_info( joined["index_right"] = None if _SOURCE_BUILDING_INDEX_COL in joined.columns: + # Fallback cells are matched exactly to their source building, so extra + # sjoin matches only duplicate the cell's row and must be dropped. All + # operations are positional: the sjoin can duplicate index labels when a + # cell contains several building points, which breaks label alignment. + source_mask = joined[_SOURCE_BUILDING_INDEX_COL].notna().to_numpy() + redundant = source_mask & joined.index.duplicated(keep="first") + if redundant.any(): + joined = joined.loc[~redundant] + source_mask = joined[_SOURCE_BUILDING_INDEX_COL].notna().to_numpy() source_index = joined[_SOURCE_BUILDING_INDEX_COL] - joined["index_right"] = source_index.combine_first(joined["index_right"]) - source_mask = source_index.notna() + joined["index_right"] = np.where(source_mask, source_index, joined["index_right"]) for column in building_columns: joined.loc[source_mask, column] = ( source_index.loc[source_mask] diff --git a/city2graph/utils.py b/city2graph/utils.py index 73c03eb..76df1f8 100644 --- a/city2graph/utils.py +++ b/city2graph/utils.py @@ -4496,7 +4496,18 @@ def _repair_or_drop_degenerate_enclosures( "grid_size=1e-3.", len(broken), ) - tessellation = run_tessellation(**{**overrides, "grid_size": 1e-3}) + retry_overrides = {**overrides, "grid_size": 1e-3} + try: + tessellation = run_tessellation(**retry_overrides) + except TypeError as e: + if "incorrect geometry type" not in str(e) or "simplify" in kwargs: + raise + logger.warning( + "Tessellation retry failed boundary simplification (%s); retrying with " + "simplify=False.", + e, + ) + tessellation = run_tessellation(**{**retry_overrides, "simplify": False}) broken = _overfilled_enclosures(tessellation, enclosures) if broken: logger.warning( diff --git a/tests/test_graph.py b/tests/test_graph.py index 977acda..fdf0a85 100644 --- a/tests/test_graph.py +++ b/tests/test_graph.py @@ -2315,12 +2315,38 @@ def test_osmnx_shaped_multigraph_gdfs_round_trip_deduplicates_symmetrized_edges( ) def test_osmnx_downloaded_multidigraph_smoke(self) -> None: - """Downloaded OSMnx MultiDiGraph round-trips through PyG with keyed edges.""" - graph = ox.graph_from_bbox( - (-0.128, 51.501, -0.124, 51.503), - network_type="drive", - simplify=True, - retain_all=False, + """OSMnx MultiDiGraph round-trips through PyG with keyed edges.""" + graph = nx.MultiDiGraph() + graph.graph["crs"] = "EPSG:4326" + graph.add_node(101, x=-0.128, y=51.501) + graph.add_node(202, x=-0.126, y=51.502) + graph.add_node(303, x=-0.124, y=51.503) + graph.add_edge( + 101, + 202, + key=0, + osmid=10, + length=100.0, + geometry=LineString([(-0.128, 51.501), (-0.126, 51.502)]), + oneway=True, + ) + graph.add_edge( + 202, + 303, + key=0, + osmid=11, + length=110.0, + geometry=LineString([(-0.126, 51.502), (-0.124, 51.503)]), + oneway=True, + ) + graph.add_edge( + 202, + 101, + key=0, + osmid=12, + length=100.0, + geometry=LineString([(-0.126, 51.502), (-0.128, 51.501)]), + oneway=True, ) nodes, edges = ox.graph_to_gdfs(graph) diff --git a/tests/test_morphology.py b/tests/test_morphology.py index 24e1672..7cbd45e 100644 --- a/tests/test_morphology.py +++ b/tests/test_morphology.py @@ -431,6 +431,48 @@ def test_include_unenclosed_buildings_is_opt_in( assert len(opt_in_nodes["place"]) == 2 assert "fallback_1" in opt_in_nodes["place"].index + def test_keep_buildings_with_fallback_and_multi_building_cell( + self, + sample_crs: str, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """Fallback cells coexist with cells containing several buildings.""" + buildings = gpd.GeoDataFrame( + {"building_id": ["outer", "nested", "unenclosed"]}, + geometry=[ + Polygon([(0, 0), (1, 0), (1, 1), (0, 1)]), + Polygon([(0.25, 0.25), (0.75, 0.25), (0.75, 0.75), (0.25, 0.75)]), + Polygon([(10, 0), (11, 0), (11, 1), (10, 1)]), + ], + crs=sample_crs, + ) + segments = gpd.GeoDataFrame( + geometry=[LineString([(-1, -1), (2, -1)])], + crs=sample_crs, + ) + + monkeypatch.setattr( + "city2graph.morphology.create_tessellation", + _enclosed_cell_or_fail_on_fallback, + ) + monkeypatch.setattr( + "city2graph.morphology._filter_adjacent_tessellation", + _passthrough_adjacent_filter, + ) + + nodes, _ = morphological_graph( + buildings, + segments, + keep_buildings=True, + include_unenclosed_buildings=True, + ) + + place_ids = list(nodes["place"].index) + # The enclosed cell holds both overlapping buildings; the fallback cell + # for the unenclosed building is matched exactly once to its source. + assert place_ids.count("fallback_2") == 1 + assert place_ids.count("enclosed") == 2 + def test_morphological_graph_passes_tessellation_limit( self, sample_crs: str, diff --git a/tests/test_utils.py b/tests/test_utils.py index 04bdfdd..f851146 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -363,6 +363,57 @@ def fake_enclosed_tessellation(**kwargs: object) -> gpd.GeoDataFrame: assert result.geometry.area.sum() == pytest.approx(enclosure.area) assert "overlapping cells" in caplog.text + def test_enclosed_tessellation_overlap_retry_falls_back_to_simplify_false( + self, + sample_buildings_gdf: gpd.GeoDataFrame, + sample_segments_gdf: gpd.GeoDataFrame, + caplog: pytest.LogCaptureFixture, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """A geometry-type error during the overlap retry should retry simplify=False.""" + enclosure = Polygon([(0, 0), (10, 0), (10, 10), (0, 10)]) + monkeypatch.setattr( + momepy, + "enclosures", + lambda **_kwargs: gpd.GeoDataFrame( + {"eID": [0]}, + geometry=[enclosure], + crs=sample_buildings_gdf.crs, + ), + ) + + sane_cells = [ + Polygon([(0, 0), (5, 0), (5, 10), (0, 10)]), + Polygon([(5, 0), (10, 0), (10, 10), (5, 10)]), + ] + + def fake_enclosed_tessellation(**kwargs: object) -> gpd.GeoDataFrame: + # The first run silently degenerates; the coarser grid_size retry + # snaps cells into non-polygonal geometry and blows up in + # coverage_simplify; only simplify=False succeeds. + if kwargs.get("grid_size") and kwargs.get("simplify") is not False: + msg = "One of the Geometry inputs is of incorrect geometry type." + raise TypeError(msg) + cells = sane_cells if kwargs.get("grid_size") else [enclosure, enclosure] + return gpd.GeoDataFrame( + {"enclosure_index": [0, 0]}, + geometry=cells, + index=[0, 1], + crs=sample_buildings_gdf.crs, + ) + + monkeypatch.setattr(momepy, "enclosed_tessellation", fake_enclosed_tessellation) + + with caplog.at_level("WARNING"): + result = utils.create_tessellation( + sample_buildings_gdf, + primary_barriers=sample_segments_gdf, + ) + + assert len(result) == 2 + assert result.geometry.area.sum() == pytest.approx(enclosure.area) + assert "retrying with simplify=False" in caplog.text + def test_enclosed_tessellation_drops_persistently_overlapping_enclosures( self, sample_buildings_gdf: gpd.GeoDataFrame,