From 516b3a5b557972f1c6ac7715b1943590b013fa41 Mon Sep 17 00:00:00 2001 From: Aduneer <249940941+Aduneer@users.noreply.github.com> Date: Sun, 9 Aug 2026 20:27:19 +0200 Subject: [PATCH] Feat: per-destination cycling mode, routed over its own bike network - destinations take "mode": "walk" | "bike", optional and defaulting to walk, so every existing config scores identically. A 35-minute walk is often a 12-minute cycle, which changes which flats look good. --- .github/workflows/ci.yml | 4 +- FlatScorer.py | 214 +++++++++++++++++++++++++++------ README.md | 99 ++++++++++----- config.example.json | 3 + pyproject.toml | 2 +- streamlit_app.py | 44 ++++++- tests/test_gui.py | 63 ++++++++++ tests/test_scoring.py | 252 ++++++++++++++++++++++++++++++++++++--- 8 files changed, 592 insertions(+), 89 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index efe87a3..41f7179 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -50,7 +50,7 @@ jobs: G.add_node(2, x=13.01, y=52.0) G.add_edge(1, 2, length=1000.0) - minutes, route = FlatScorer.walk_route(G, (52.0, 13.0), (52.0, 13.01)) + minutes, route = FlatScorer.route_time(G, (52.0, 13.0), (52.0, 13.01)) assert route == [(52.0, 13.0), (52.0, 13.01)], route print(f"routed OK: {minutes:.2f} min") PY @@ -95,7 +95,7 @@ jobs: G.add_node(2, x=13.01, y=52.0) G.add_edge(1, 2, length=1000.0) - minutes, route = FlatScorer.walk_route(G, (52.0, 13.0), (52.0, 13.01)) + minutes, route = FlatScorer.route_time(G, (52.0, 13.0), (52.0, 13.01)) assert route == [(52.0, 13.0), (52.0, 13.01)], route print(f"routed OK: {minutes:.2f} min") PY diff --git a/FlatScorer.py b/FlatScorer.py index 36c4c18..de40eb1 100755 --- a/FlatScorer.py +++ b/FlatScorer.py @@ -3,9 +3,9 @@ FlatScorer — Multi-criteria apartment scoring tool. Scores candidate apartments based on nearby amenities, transit access, -green space, road-noise proximity, walking commute to user-defined -destinations, and rent — producing a ranked comparison table, CSV -export, interactive Folium map, and weight-sensitivity analysis. +green space, road-noise proximity, walking or cycling commute to +user-defined destinations, and rent — producing a ranked comparison table, +CSV export, interactive Folium map, and weight-sensitivity analysis. Usage: python FlatScorer.py --generate-config config.json @@ -108,6 +108,48 @@ # only mean what they say together - lower this and every commute term drops. DEFAULT_WALKING_SPEED_M_PER_MIN = 83.33 +# Assumed cycling pace, in metres per minute: 250 is 15 km/h, the usual planning +# figure for urban cycling *including* junctions, lights and locking up - not the +# speed a fit rider holds on a clear path. Same relationship to `commute_cap_min` +# as the walking pace above. +DEFAULT_CYCLING_SPEED_M_PER_MIN = 250.0 + +# Travel modes a destination may declare via `"mode"`. Each entry names +# everything that is mode-specific about routing a commute, so adding a mode is a +# table entry rather than a branch in `run()`: +# network_type - the OSMnx street network to download for it +# speed_param - the `parameters` key holding its pace, which is also the +# FlatScorer attribute the pace is read back from +# column_suffix - what the destination's minutes column is called, so a +# cycling commute is never reported in a column saying "walk" +# label/verb - wording for the run log and the map popups +# +# The networks genuinely differ - the walk graph carries footways bikes may not +# use and drops roads they may - so a mode always routes over its own graph. A +# bike time computed on the walk network would be wrong in both directions at +# once, and wrong invisibly. +TRAVEL_MODES = { + "walk": { + "network_type": "walk", + "speed_param": "walking_speed_m_per_min", + "column_suffix": "walk_min", + "label": "walking", + "verb": "walk", + }, + "bike": { + "network_type": "bike", + "speed_param": "cycling_speed_m_per_min", + "column_suffix": "bike_min", + "label": "cycling", + "verb": "cycle", + }, +} + +# What a destination that doesn't declare a mode gets. Every config written +# before cycling existed is an all-walk config, and has to keep scoring +# identically - including paying for exactly one street-network download. +DEFAULT_TRAVEL_MODE = "walk" + DEFAULT_WEIGHTS = { "supermarket": 0.30, "bakery": 0.10, @@ -156,12 +198,14 @@ "White House": { "address": "1600 Pennsylvania Ave NW, Washington, DC 20500, USA", "weight": 0.15, + "mode": "walk", "icon": "landmark", "color": "blue" }, "Union Station": { "address": "50 Massachusetts Ave NE, Washington, DC 20002, USA", "weight": 0.15, + "mode": "walk", "icon": "train", "color": "red" } @@ -173,6 +217,7 @@ "rent_budget_eur": DEFAULT_RENT_BUDGET_EUR, "commute_cap_min": DEFAULT_COMMUTE_CAP_MIN, "walking_speed_m_per_min": DEFAULT_WALKING_SPEED_M_PER_MIN, + "cycling_speed_m_per_min": DEFAULT_CYCLING_SPEED_M_PER_MIN, "poi_dedupe_tolerance_m": DEFAULT_POI_DEDUPE_TOLERANCE_M, "max_bbox_span_km": DEFAULT_MAX_BBOX_SPAN_KM, "saturation": dict(DEFAULT_SATURATION), @@ -246,6 +291,38 @@ def _as_number(value: Any) -> float | None: return None +def destination_mode(info: Any) -> str: + """The travel mode a destination declares, defaulting to walking. + + `validate_config` rejects anything outside `TRAVEL_MODES` before `run()` + reaches this, so the fallback for an unknown value only covers callers that + skipped validation - it keeps the map and the routing loop agreeing on one + answer rather than raising halfway through a scored run. + """ + if not isinstance(info, dict): + return DEFAULT_TRAVEL_MODE + mode = info.get("mode", DEFAULT_TRAVEL_MODE) + return mode if mode in TRAVEL_MODES else DEFAULT_TRAVEL_MODE + + +def commute_column(dest_name: str, mode: str = DEFAULT_TRAVEL_MODE) -> str: + """Name of the table/CSV column carrying a destination's commute minutes. + + The mode is part of the name, so a cycling commute is never reported in a + column called `..._walk_min`. An all-walk config keeps exactly the columns it + had before cycling existed. + """ + suffix = TRAVEL_MODES.get(mode, TRAVEL_MODES[DEFAULT_TRAVEL_MODE])["column_suffix"] + return f"{dest_name.lower().replace(' ', '_')}_{suffix}" + + +# Every suffix `commute_column` can produce, longest first so a shorter suffix +# can't strip a prefix of a longer one when a label is recovered from a column. +COMMUTE_COLUMN_SUFFIXES = tuple( + sorted((f"_{spec['column_suffix']}" for spec in TRAVEL_MODES.values()), key=len, reverse=True) +) + + def _validate_candidate(index: int, candidate: Any, problems: list[str], seen_names: dict[str, int]): """Check one candidate entry, appending any problems found.""" label = f"candidates[{index}]" @@ -323,6 +400,11 @@ def validate_config(config: Any) -> list[str]: problems.append(f"{label}: 'weight' must be a number, got {dest_info['weight']!r}") elif weight < 0: problems.append(f"{label}: 'weight' is negative ({weight:g}); weights are relative importances and cannot be below 0") + # An unrecognised mode can't be guessed at: silently walking a + # destination the user meant to cycle would report a commute two to + # three times too long and quietly demote every flat near it. + if "mode" in dest_info and dest_info["mode"] not in TRAVEL_MODES: + problems.append(f"{label}: 'mode' must be one of {', '.join(sorted(TRAVEL_MODES))}, got {dest_info['mode']!r}") weights = config.get("weights", {}) if not isinstance(weights, dict): @@ -355,7 +437,7 @@ def validate_config(config: Any) -> list[str]: # limit; a zero or negative value silently zeroes or inverts the term it # governs, or (for max_bbox_span_km) rejects every possible search. for key in ("buffer_m", "noise_cap_m", "rent_budget_eur", "commute_cap_min", "max_bbox_span_km", - "walking_speed_m_per_min"): + "walking_speed_m_per_min", "cycling_speed_m_per_min"): if key not in params: continue number = _as_number(params[key]) @@ -694,20 +776,27 @@ def nearest_node(G: nx.MultiDiGraph, point: tuple[float, float]): return ox.distance.nearest_nodes(G, point[1], point[0]) -def walk_route(G: nx.MultiDiGraph, orig: tuple[float, float], dest: tuple[float, float], walking_speed_m_per_min: float = DEFAULT_WALKING_SPEED_M_PER_MIN, projected_crs: str | None = None, +def route_time(G: nx.MultiDiGraph, orig: tuple[float, float], dest: tuple[float, float], speed_m_per_min: float = DEFAULT_WALKING_SPEED_M_PER_MIN, projected_crs: str | None = None, orig_node=None, dest_node=None) -> tuple[float, list[tuple[float, float]]]: - """Calculate walking time in minutes and the shortest-path route (list of lat/lon points) between two coordinates over OSM graph G. + """Calculate travel time in minutes and the shortest-path route (list of lat/lon points) between two coordinates over OSM graph G. + + Nothing here is mode-specific: the network to route over and the pace to + divide by both arrive as arguments, so the same function serves walking and + cycling. It is the caller's job to pass a graph and a speed that agree with + each other - a cycling pace over the pedestrian network is not a bike time. The speed defaults to `DEFAULT_WALKING_SPEED_M_PER_MIN` so the function stays - usable standalone, but `run()` always passes the configured - `parameters['walking_speed_m_per_min']` - the default is a fallback, not the - value the tool actually scores with. + usable standalone, but `run()` always passes the pace configured for the + destination's mode - the default is a fallback, not the value the tool + actually scores with. `orig_node`/`dest_node` let a caller supply an already-resolved graph node. `run()` does, because the endpoints repeat: without it the lookup runs 2*candidates*destinations times where candidates+destinations would do, and on a city-sized graph that lookup is not cheap. Passing them is purely an - optimisation - omit them and the same nodes are resolved here. + optimisation - omit them and the same nodes are resolved here. They must come + from *this* graph: a node id resolved against another mode's network names a + different place, which yields a plausible number rather than an error. """ if orig_node is None: orig_node = nearest_node(G, orig) @@ -717,11 +806,11 @@ def walk_route(G: nx.MultiDiGraph, orig: tuple[float, float], dest: tuple[float, path = nx.shortest_path(G, orig_node, dest_node, weight="length") length_m = nx.path_weight(G, path, weight="length") route = [(G.nodes[n]["y"], G.nodes[n]["x"]) for n in path] - return length_m / walking_speed_m_per_min, route + return length_m / speed_m_per_min, route except nx.NetworkXNoPath: - print(f"[!] Warning: No walking path found between {orig} and {dest}. Defaulting to straight-line distance.") + print(f"[!] Warning: No route found between {orig} and {dest}. Defaulting to straight-line distance.") dist_m = straight_line_distance_m(orig, dest, projected_crs) - return dist_m / walking_speed_m_per_min, [orig, dest] + return dist_m / speed_m_per_min, [orig, dest] class FlatScorer: @@ -743,7 +832,10 @@ def __init__(self, config: dict[str, Any], verbose: bool = True): self.commute_cap_min = self.params.get("commute_cap_min", DEFAULT_COMMUTE_CAP_MIN) self.poi_dedupe_tolerance_m = self.params.get("poi_dedupe_tolerance_m", DEFAULT_POI_DEDUPE_TOLERANCE_M) self.max_bbox_span_km = self.params.get("max_bbox_span_km", DEFAULT_MAX_BBOX_SPAN_KM) + # Named to match TRAVEL_MODES[...]["speed_param"], which is how + # `mode_speed()` finds the right one without a lookup table of its own. self.walking_speed_m_per_min = self.params.get("walking_speed_m_per_min", DEFAULT_WALKING_SPEED_M_PER_MIN) + self.cycling_speed_m_per_min = self.params.get("cycling_speed_m_per_min", DEFAULT_CYCLING_SPEED_M_PER_MIN) # Per-metric half-credit points; a config may override any subset. self.saturation = dict(DEFAULT_SATURATION, **self.params.get("saturation", {})) self.configured_crs = self.params.get("projected_crs", "auto") @@ -759,6 +851,11 @@ def _log(self, msg: str): if self.verbose: print(msg) + def mode_speed(self, mode: str) -> float: + """The configured pace, in m/min, for one travel mode.""" + spec = TRAVEL_MODES.get(mode, TRAVEL_MODES[DEFAULT_TRAVEL_MODE]) + return float(getattr(self, spec["speed_param"])) + def resolve_crs(self, lats: list[float], lons: list[float]) -> str: """Determine projected CRS (e.g. UTM zone) for metric calculations.""" if self.configured_crs and self.configured_crs.lower() != "auto": @@ -995,9 +1092,16 @@ def run(self) -> pd.DataFrame: centre_labels=candidate_points) self._log(f"[+] Search area spans {span_km:.1f} km (limit {self.max_bbox_span_km:g} km)") - self._log("\nDownloading street walking network from OpenStreetMap...") - def get_graph(): - return ox.graph_from_bbox(bbox=bbox, network_type="walk") + # One street network per travel mode actually used, in the order the + # destinations first mention them. Downloading lazily is what keeps an + # all-walk config - every config that predates cycling, including the + # shipped example - paying for exactly the one download it always paid + # for; only a genuinely mixed config pays for a second. + dest_modes = { + dest_name: destination_mode(data["info"]) + for dest_name, data in resolved_destinations.items() + } + modes_in_use = list(dict.fromkeys(dest_modes.values())) def get_pois(): tags = { @@ -1010,7 +1114,18 @@ def get_pois(): } return ox.features_from_bbox(bbox=bbox, tags=tags) - G = query_with_retry(get_graph) + graphs = {} + for mode in modes_in_use: + spec = TRAVEL_MODES[mode] + self._log(f"\nDownloading the {spec['label']} street network from OpenStreetMap...") + # network_type is bound as a default rather than closed over, so the + # lambda can't be caught out by the loop variable moving on. + graphs[mode] = query_with_retry( + lambda network_type=spec["network_type"]: ox.graph_from_bbox(bbox=bbox, network_type=network_type) + ) + if not modes_in_use: + self._log("\nNo destinations to route to, so no street network is needed.") + self._log("Downloading points of interest (POIs) from OpenStreetMap...") pois = query_with_retry(get_pois) @@ -1056,15 +1171,24 @@ def dedupe(gdf, label, keep="points"): # Each destination's nearest graph node is the same for every candidate, # so resolve it once here instead of once per (candidate, destination). + # Keyed by (mode, destination), never by destination alone: a node id is + # only meaningful in the graph it came from, and the same id names a + # different junction in the cycling network. Reusing one across modes + # produces a plausible commute time rather than an error, so nothing + # downstream would notice. dest_nodes = { - dest_name: nearest_node(G, data["coords"]) + (dest_modes[dest_name], dest_name): nearest_node(graphs[dest_modes[dest_name]], data["coords"]) for dest_name, data in resolved_destinations.items() } - # Worth stating: it converts every routed distance into the minutes that - # commute_cap_min judges, so a reader comparing two runs needs to know it. - self._log(f"\nScoring candidates (walking at {self.walking_speed_m_per_min:g} m/min " - f"= {self.walking_speed_m_per_min * 60 / 1000:.1f} km/h)...") + # Worth stating: these convert every routed distance into the minutes that + # commute_cap_min judges, so a reader comparing two runs needs to know them. + paces = ", ".join( + f"{TRAVEL_MODES[mode]['label']} at {self.mode_speed(mode):g} m/min " + f"= {self.mode_speed(mode) * 60 / 1000:.1f} km/h" + for mode in modes_in_use + ) + self._log(f"\nScoring candidates ({paces})..." if paces else "\nScoring candidates...") metrics_by_name = {} routes_by_candidate = {} rows = [] @@ -1072,7 +1196,10 @@ def dedupe(gdf, label, keep="points"): for name, info in resolved_candidates.items(): lat, lon = info["coords"] rent = info["rent"] - orig_node = nearest_node(G, (lat, lon)) + # Per mode for the same reason the destination cache is: this flat's + # nearest walking junction and nearest cycling junction are different + # nodes in different graphs. + orig_nodes = {mode: nearest_node(graphs[mode], (lat, lon)) for mode in modes_in_use} green_area_m2, green_points = green_area_and_points(lat, lon, green_p, projected_crs, dist=self.buffer_m) dist_to_busy_road = nearest_distance_m(lat, lon, roads_p, projected_crs) @@ -1082,11 +1209,12 @@ def dedupe(gdf, label, keep="points"): dest_routes = {} for dest_name, dest_data in resolved_destinations.items(): dest_coords = dest_data["coords"] - dest_times[dest_name], dest_routes[dest_name] = walk_route( - G, (lat, lon), dest_coords, - walking_speed_m_per_min=self.walking_speed_m_per_min, + mode = dest_modes[dest_name] + dest_times[dest_name], dest_routes[dest_name] = route_time( + graphs[mode], (lat, lon), dest_coords, + speed_m_per_min=self.mode_speed(mode), projected_crs=projected_crs, - orig_node=orig_node, dest_node=dest_nodes[dest_name], + orig_node=orig_nodes[mode], dest_node=dest_nodes[(mode, dest_name)], ) routes_by_candidate[name] = dest_routes @@ -1118,8 +1246,7 @@ def dedupe(gdf, label, keep="points"): } for dest_name, mins in dest_times.items(): - clean_dest_key = dest_name.lower().replace(" ", "_") - row[f"{clean_dest_key}_walk_min"] = round(mins, 1) + row[commute_column(dest_name, dest_modes[dest_name])] = round(mins, 1) row["lat"] = lat row["lon"] = lon @@ -1144,12 +1271,18 @@ def dedupe(gdf, label, keep="points"): return df def generate_map(self, df: pd.DataFrame, resolved_destinations: dict[str, Any], html_file: str, routes_by_candidate: dict[str, dict[str, list[tuple[float, float]]]] | None = None): - """Generate interactive Folium map with candidate apartments, destination pins, and predicted walking routes.""" + """Generate interactive Folium map with candidate apartments, destination pins, and predicted commute routes.""" routes_by_candidate = routes_by_candidate or {} first_lat = df.iloc[0]["lat"] first_lon = df.iloc[0]["lon"] m_map = folium.Map(location=[first_lat, first_lon], zoom_start=13) - route_group = folium.FeatureGroup(name="Predicted walking routes", show=self.show_walk_routes) + + dest_modes = {name: destination_mode(data["info"]) for name, data in resolved_destinations.items()} + # An all-walk map keeps the layer name it has always had; only a map that + # actually mixes modes needs the broader wording. + layer_name = ("Predicted walking routes" if set(dest_modes.values()) <= {"walk"} + else "Predicted commute routes") + route_group = folium.FeatureGroup(name=layer_name, show=self.show_walk_routes) # Add destinations to map for dest_name, dest_data in resolved_destinations.items(): @@ -1184,9 +1317,12 @@ def generate_map(self, df: pd.DataFrame, resolved_destinations: dict[str, Any], dest_lines = [] for col in df.columns: - if col.endswith("_walk_min"): - dest_label = col.replace("_walk_min", "").replace("_", " ").title() - dest_lines.append(f"{dest_label}: {row[col]} min") + suffix = next((s for s in COMMUTE_COLUMN_SUFFIXES if col.endswith(s)), None) + if suffix is None: + continue + dest_label = col[:-len(suffix)].replace("_", " ").title() + verb = next(spec["verb"] for spec in TRAVEL_MODES.values() if suffix == f"_{spec['column_suffix']}") + dest_lines.append(f"{dest_label}: {row[col]} min {verb}") dest_html = " | ".join(dest_lines) popup = ( @@ -1210,14 +1346,18 @@ def generate_map(self, df: pd.DataFrame, resolved_destinations: dict[str, Any], for dest_name, route_coords in routes_by_candidate.get(row["name"], {}).items(): if not route_coords or len(route_coords) < 2: continue - dest_key = dest_name.lower().replace(" ", "_") - mins = row.get(f"{dest_key}_walk_min") + mode = dest_modes.get(dest_name, DEFAULT_TRAVEL_MODE) + mins = row.get(commute_column(dest_name, mode)) folium.PolyLine( locations=route_coords, color=color, weight=3, opacity=0.6, - tooltip=f"{row['name']} → {dest_name}: {mins} min", + # Lines are coloured by candidate score, so on a mixed map the + # dashes are the only thing separating a cycled leg from a + # walked one. + dash_array="8" if mode != DEFAULT_TRAVEL_MODE else None, + tooltip=f"{row['name']} → {dest_name}: {mins} min {TRAVEL_MODES[mode]['verb']}", ).add_to(route_group) route_group.add_to(m_map) diff --git a/README.md b/README.md index 4d05b5e..78f8c5d 100644 --- a/README.md +++ b/README.md @@ -105,19 +105,50 @@ this is just a friendlier way to build the config and view the output. For each candidate apartment, FlatScorer: 1. **Geocodes** all addresses via Nominatim (through OSMnx). -2. **Downloads** the walking street network and points of interest for the - bounding region from the Overpass API (with automatic mirror failover). +2. **Downloads** a street network per travel mode your destinations actually use + (pedestrian, cycling, or both) plus the points of interest for the bounding + region, from the Overpass API (with automatic mirror failover). 3. **Deduplicates POIs** mapped both as a node and as a building outline (see [Duplicate POIs](#duplicate-pois)). 4. **Counts nearby amenities** within a configurable radius (default 500 m): supermarkets, bakeries, pharmacies, gyms, bus/tram stops. 5. **Measures green space** — park and forest polygon area plus point features. 6. **Estimates noise exposure** via distance to the nearest primary/secondary road. -7. **Routes walking commutes** to each of your defined destinations over the - real pedestrian network (~5 km/h). +7. **Routes commutes** to each of your defined destinations over the real + network for that destination's travel mode — walking (~5 km/h) or cycling + (~15 km/h), see [Travel modes](#travel-modes). 8. **Normalizes every metric** onto a common 0–1 scale. 9. **Computes a weighted average** of those normalized values, on a 0–10 scale. +### Travel modes + +A destination declares how you get there with `"mode": "walk"` (the default) or +`"mode": "bike"`. It changes the answer substantially: a 35-minute walk is often +a 12-minute cycle, and which flats look good depends on which of those you meant. + +```jsonc +"destinations": { + "Office": { "address": "...", "weight": 0.20, "mode": "bike" }, + "Supermarket":{ "address": "...", "weight": 0.10 } // walks, as before +} +``` + +Each mode routes over **its own** street network, downloaded separately. That is +deliberate and not negotiable: the pedestrian graph carries footways a bike may +not use and drops roads it may, so a cycling time computed on it would be wrong +in both directions at once — and wrong invisibly, since it still produces a +plausible number. + +The networks are downloaded lazily, one per mode your destinations actually +mention. An all-walk config — every config written before this existed, and the +shipped example — makes exactly one download, as it always did. Only a genuinely +mixed config pays for a second. + +Each mode has its own pace (`walking_speed_m_per_min`, `cycling_speed_m_per_min`) +and its own column in the results: `office_bike_min` beside `supermarket_walk_min`, +so a cycled commute is never reported under a heading that says "walk". On the map, +cycled legs are drawn dashed. + ### Duplicate POIs OpenStreetMap frequently maps one real place twice — a supermarket tagged on a @@ -195,7 +226,7 @@ thing in every run — and a field of similar flats is allowed to look similar. |---|---| | Terminal table | Ranked summary printed to stdout | | `apartment_scores.csv` | Full metrics for every candidate | -| `apartment_map.html` | Interactive Folium map with color-coded pins and predicted walking routes | +| `apartment_map.html` | Interactive Folium map with color-coded pins and predicted commute routes | | Sensitivity report | ±20% weight perturbation check on ranking stability | ## Configuration @@ -214,11 +245,12 @@ Everything is driven by a single JSON file. Generate a template with } ], - // Places you commute to — each gets a walking-time column + // Places you commute to — each gets a travel-time column "destinations": { "Office": { "address": "Alexanderplatz 1, 10178 Berlin, Germany", "weight": 0.20, // importance, relative to the weights below + "mode": "bike", // "walk" (default) or "bike" — see Travel modes "icon": "briefcase", // FontAwesome icon on the map "color": "blue" } @@ -241,15 +273,16 @@ Everything is driven by a single JSON file. Generate a template with "buffer_m": 500, // amenity search radius in meters "noise_cap_m": 200, // quiet term maxes out at this distance from a busy road "rent_budget_eur": 2500, // rent at/above this scores 0 on the rent term - "commute_cap_min": 45, // a walk this long scores 0 for that destination + "commute_cap_min": 45, // a commute this long scores 0 for that destination "walking_speed_m_per_min": 83.33, // assumed pace; 83.33 m/min = 5 km/h + "cycling_speed_m_per_min": 250, // assumed pace; 250 m/min = 15 km/h "max_bbox_span_km": 30, // refuse to download an area wider than this "saturation": { // count earning half credit (diminishing returns) "supermarket": 2, "bakery": 2, "pharmacy": 1, "gym": 1, "transit": 4, "green": 30 }, "projected_crs": "auto", // auto-detect UTM zone, or e.g. "EPSG:25832" - "show_walk_routes": true // draw predicted walking routes on the map by default + "show_walk_routes": true // draw predicted commute routes on the map by default }, "output": { @@ -266,21 +299,25 @@ Everything is driven by a single JSON file. Generate a template with linear. Set it to the top of your budget; setting it far above your actual range flattens the differences between candidates. -- **`commute_cap_min`** — The walk length at which a destination stops earning +- **`commute_cap_min`** — The travel time at which a destination stops earning anything. Same shape as the rent term, including the same tradeoff: everything - past the cap scores 0, so a 50-minute walk and a 90-minute walk are - indistinguishable on that term. If the score breakdown shows a destination at - 0.00 for every candidate, raise the cap (or accept that nobody is walking - there). Both anchors are deliberately absolute — that is what makes a score - mean the same thing between runs. - -- **`walking_speed_m_per_min`** — The pace every routed distance is divided by to - get minutes. The default 83.33 m/min is 5 km/h, the usual planning figure for - an unhurried adult on the flat; 100 m/min (6 km/h) is a brisk walker. It only - means anything alongside `commute_cap_min`, because the two multiply out: a - slower pace makes every walk longer in minutes and so pushes more destinations - towards the cap. If you change one, sanity-check the other — the routed - distances themselves have not moved. + past the cap scores 0, so a 50-minute commute and a 90-minute commute are + indistinguishable on that term. One cap covers every mode — it is how long you + are willing to travel, not how far. If the score breakdown shows a destination + at 0.00 for every candidate, raise the cap, switch that destination to `bike`, + or accept that nobody is getting there. Both anchors are deliberately absolute + — that is what makes a score mean the same thing between runs. + +- **`walking_speed_m_per_min` / `cycling_speed_m_per_min`** — The pace each mode's + routed distances are divided by to get minutes. The walking default 83.33 m/min + is 5 km/h, the usual planning figure for an unhurried adult on the flat; 100 + m/min (6 km/h) is a brisk walker. The cycling default 250 m/min is 15 km/h, + urban cycling *including* junctions, lights and locking up — not the speed a fit + rider holds on a clear path, which is why it is well under what a bike computer + reports. Both only mean anything alongside `commute_cap_min`, because they + multiply out: a slower pace makes every commute longer in minutes and so pushes + more destinations towards the cap. If you change one, sanity-check the other — + the routed distances themselves have not moved. - **`saturation`** — The count that earns half credit for each amenity, i.e. how quickly more of something stops helping. Lower is easier to satisfy: at @@ -313,11 +350,16 @@ Everything is driven by a single JSON file. Generate a template with destination at 0.15 against weights totalling 1.5 controls 10% of the score — at most 1 point out of 10, earned by living next door to it. -- **`show_walk_routes`** — Whether the map's "Predicted walking routes" layer - starts visible. The routes trace each candidate's actual shortest path over - the OSM pedestrian network to every destination (color-matched to that - candidate's score) and can always be toggled via the map's layer control - regardless of this setting. +- **Destination `mode`** — `"walk"` (the default) or `"bike"`, deciding which + street network this commute is routed over and which pace it is divided by. + See [Travel modes](#travel-modes); omitting it walks, so every config written + before cycling existed scores exactly as it did. + +- **`show_walk_routes`** — Whether the map's predicted-routes layer starts + visible. The routes trace each candidate's actual shortest path to every + destination over that destination's own network (color-matched to the + candidate's score, dashed for cycled legs) and can always be toggled via the + map's layer control regardless of this setting. ### Configuration is validated before anything runs @@ -340,7 +382,8 @@ doesn't score neutrally — it scores *perfectly* on that term and tends to win. So a missing or non-positive rent is rejected rather than guessed at. The other checks cover missing names and addresses, duplicate candidate names (they'd silently overwrite each other), non-numeric or negative weights, an all-zero -weight vector, and non-positive normalization anchors. +weight vector, non-positive normalization anchors, and an unrecognised +destination travel `mode`. In the GUI the same problems appear on the Run page and the run button stays disabled until they're fixed. diff --git a/config.example.json b/config.example.json index 6a36048..2d9d292 100644 --- a/config.example.json +++ b/config.example.json @@ -20,12 +20,14 @@ "White House": { "address": "1600 Pennsylvania Ave NW, Washington, DC 20500, USA", "weight": 0.15, + "mode": "walk", "icon": "landmark", "color": "blue" }, "Union Station": { "address": "50 Massachusetts Ave NE, Washington, DC 20002, USA", "weight": 0.15, + "mode": "walk", "icon": "train", "color": "red" } @@ -46,6 +48,7 @@ "rent_budget_eur": 2500, "commute_cap_min": 45, "walking_speed_m_per_min": 83.33, + "cycling_speed_m_per_min": 250, "poi_dedupe_tolerance_m": 10, "max_bbox_span_km": 30, "saturation": { diff --git a/pyproject.toml b/pyproject.toml index a5c31fc..09b3303 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -5,7 +5,7 @@ build-backend = "setuptools.build_meta" [project] name = "flatscorer" version = "0.1.0" -description = "Score and compare candidate apartments using OpenStreetMap data — amenities, transit, green space, road noise, walking commutes, and rent." +description = "Score and compare candidate apartments using OpenStreetMap data — amenities, transit, green space, road noise, walking and cycling commutes, and rent." readme = "README.md" requires-python = ">=3.9" license = "MIT" diff --git a/streamlit_app.py b/streamlit_app.py index f968cda..5183d3d 100755 --- a/streamlit_app.py +++ b/streamlit_app.py @@ -30,8 +30,10 @@ from FlatScorer import ( DEFAULT_CONFIG, DEFAULT_DEST_WEIGHT, + DEFAULT_TRAVEL_MODE, NARROW_MARGIN_THRESHOLD, SCORE_SCALE_MAX, + TRAVEL_MODES, FlatScorer, SearchAreaError, validate_config, @@ -60,6 +62,11 @@ DEFAULT_WEIGHTS = DEFAULT_CONFIG["weights"] DEFAULT_PARAMS = DEFAULT_CONFIG["parameters"] +# Travel modes come from the engine rather than a literal here, so the dropdown +# can never offer a mode validate_config would reject. +MODE_CHOICES = list(TRAVEL_MODES) +MODE_EMOJI = {"walk": "🚶", "bike": "🚴"} + # ----------------------------------------------------------- Helper Functions -- @@ -76,6 +83,7 @@ def _init_state(): "name": name, "address": info["address"], "weight": info["weight"], + "mode": info.get("mode", DEFAULT_TRAVEL_MODE), "icon": info.get("icon", "star"), "color": info.get("color", "blue"), }) @@ -122,6 +130,7 @@ def _load_config_into_state(config: dict[str, Any]): "name": name, "address": info.get("address", ""), "weight": info.get("weight", 0.15), + "mode": info.get("mode", DEFAULT_TRAVEL_MODE), "icon": info.get("icon", "star"), "color": info.get("color", "blue"), }) @@ -159,6 +168,9 @@ def _build_config() -> dict[str, Any]: destinations[row["name"]] = { "address": row["address"], "weight": float(row.get("weight", 0.15) or 0.15), + # A cleared mode cell arrives as None/NaN; fall back rather than + # writing a mode the engine would reject. + "mode": row.get("mode") if row.get("mode") in TRAVEL_MODES else DEFAULT_TRAVEL_MODE, "icon": row.get("icon", "star") or "star", "color": row.get("color", "blue") or "blue", } @@ -653,7 +665,8 @@ def _inject_custom_theme():
📍 Commute Destinations
- Places you frequently travel to (e.g. work, university, gym). Each destination incurs a walking-time penalty scaled by its weight. + Places you frequently travel to (e.g. work, university, gym). Each destination incurs a travel-time penalty scaled by its weight, + routed over the real network for its travel mode — walking or cycling. Mixing modes adds one extra OpenStreetMap download.
""", @@ -672,6 +685,12 @@ def _inject_custom_theme(): help="Relative importance of this commute, competing in the same pool as the " "amenity weights. See the influence table on the Weights page.", ), + "mode": st.column_config.SelectboxColumn( + "Travel Mode", options=MODE_CHOICES, required=True, + help="How you get there. Each mode routes over its own street network at its own " + "pace (set on the Weights & Parameters page) — a 35-minute walk is often a " + "12-minute cycle. Using both modes costs one extra OpenStreetMap download.", + ), "icon": st.column_config.SelectboxColumn("Map Icon", options=ICON_CHOICES), "color": st.column_config.SelectboxColumn("Map Color", options=COLOR_CHOICES), }, @@ -753,7 +772,8 @@ def _inject_custom_theme(): if not dest_name: continue combined[f"dest_{dest_name}"] = float(row.get("weight", DEFAULT_DEST_WEIGHT) or 0.0) - dest_labels[f"dest_{dest_name}"] = f"🚶 Commute to {dest_name}" + mode = row.get("mode") if row.get("mode") in TRAVEL_MODES else DEFAULT_TRAVEL_MODE + dest_labels[f"dest_{dest_name}"] = f"{MODE_EMOJI.get(mode, '🚶')} Commute to {dest_name}" shares = weight_shares(combined) if sum(combined.values()) <= 0: @@ -836,7 +856,7 @@ def _inject_custom_theme(): help="Distance from a busy road at which the quiet term maxes out. Raising it no longer inflates noise's influence — the term is scaled by the cap.", ) - p5, p6, p7 = st.columns([2, 1, 1]) + p5, p6, p7, p8 = st.columns([2, 1, 1, 1]) with p5: st.session_state.params["projected_crs"] = st.text_input( "Projected CRS", @@ -862,11 +882,25 @@ def _inject_custom_theme(): ) st.session_state.params["walking_speed_m_per_min"] = walking_speed st.caption(f"≈ {walking_speed * 60 / 1000:.1f} km/h") + with p8: + cycling_speed = st.number_input( + "Cycling speed (m/min)", + min_value=1.0, + value=float(st.session_state.params.get( + "cycling_speed_m_per_min", DEFAULT_PARAMS["cycling_speed_m_per_min"])), + step=10.0, + help="Pace used for destinations set to 'bike' on the Destinations page. The default " + "250 m/min (15 km/h) is urban cycling including junctions and locking up, not " + "open-road speed.", + ) + st.session_state.params["cycling_speed_m_per_min"] = cycling_speed + st.caption(f"≈ {cycling_speed * 60 / 1000:.1f} km/h") st.session_state.params["show_walk_routes"] = st.checkbox( - "Show predicted walking routes on map by default", + "Show predicted commute routes on map by default", value=bool(st.session_state.params.get("show_walk_routes", True)), - help="Draws each candidate's shortest walking path to every destination on the map. Always toggleable via the map's layer control.", + help="Draws each candidate's shortest path to every destination on the map, over that destination's own network. " + "Cycled legs are dashed. Always toggleable via the map's layer control.", ) diff --git a/tests/test_gui.py b/tests/test_gui.py index 73b23cd..e2fb0bd 100644 --- a/tests/test_gui.py +++ b/tests/test_gui.py @@ -10,6 +10,7 @@ import os +import pandas as pd import pytest st = pytest.importorskip("streamlit") @@ -141,6 +142,68 @@ def test_the_walking_speed_widget_reaches_the_built_config(): assert params["walking_speed_m_per_min"] == 100.0 +def test_the_cycling_speed_widget_reaches_the_built_config(): + """Same contract as the walking pace: tunable only if the GUI's value scores.""" + import streamlit_app + + app = fresh_app() + app.session_state["main_nav_radio"] = WEIGHTS + app.run() + + speed_input = next(w for w in app.number_input if "Cycling speed" in w.label) + speed_input.set_value(200.0).run() + + assert not app.exception, app.exception + + with pytest.MonkeyPatch.context() as mp: + mp.setattr(streamlit_app.st, "session_state", app.session_state) + params = streamlit_app._build_config()["parameters"] + + assert params["cycling_speed_m_per_min"] == 200.0 + + +def test_the_mode_dropdown_can_only_offer_modes_the_engine_accepts(): + """A dropdown listing a mode validate_config rejects would be a dead end.""" + import streamlit_app + from FlatScorer import TRAVEL_MODES + + assert streamlit_app.MODE_CHOICES == list(TRAVEL_MODES) + + +def test_a_cycling_destination_survives_the_round_trip_into_a_config(): + import streamlit_app + from FlatScorer import validate_config + + app = fresh_app() + with pytest.MonkeyPatch.context() as mp: + mp.setattr(streamlit_app.st, "session_state", app.session_state) + app.session_state.destinations_df = pd.DataFrame([ + {"name": "Office", "address": "Alexanderplatz 1, Berlin", "weight": 0.2, + "mode": "bike", "icon": "briefcase", "color": "blue"}, + ]) + config = streamlit_app._build_config() + + assert config["destinations"]["Office"]["mode"] == "bike" + assert validate_config(config) == [] + + +def test_a_destination_row_with_no_mode_column_still_builds_a_walking_destination(): + """An uploaded pre-cycling config has no 'mode' at all — it must still run.""" + import streamlit_app + from FlatScorer import validate_config + + app = fresh_app() + with pytest.MonkeyPatch.context() as mp: + mp.setattr(streamlit_app.st, "session_state", app.session_state) + app.session_state.destinations_df = pd.DataFrame([ + {"name": "Office", "address": "Alexanderplatz 1, Berlin", "weight": 0.2}, + ]) + config = streamlit_app._build_config() + + assert config["destinations"]["Office"]["mode"] == "walk" + assert validate_config(config) == [] + + def test_editing_a_saturation_widget_does_not_mutate_the_module_default(): # `params` nests a dict, so a shallow copy of DEFAULT_PARAMS would let the # widgets rewrite the built-in defaults for the rest of the process. diff --git a/tests/test_scoring.py b/tests/test_scoring.py index 2f90aa8..630f2b3 100644 --- a/tests/test_scoring.py +++ b/tests/test_scoring.py @@ -465,24 +465,24 @@ def disconnected_graph() -> nx.MultiDiGraph: return G -def test_walk_route_fallback_uses_the_projected_crs_when_given(): - minutes, route = fs.walk_route(disconnected_graph(), (52.0, 13.0), (52.0, 14.0), +def test_route_time_fallback_uses_the_projected_crs_when_given(): + minutes, route = fs.route_time(disconnected_graph(), (52.0, 13.0), (52.0, 14.0), projected_crs=BERLIN_CRS) assert minutes == pytest.approx(68_500 / 83.33, rel=0.02) assert route == [(52.0, 13.0), (52.0, 14.0)] -def test_walk_route_fallback_without_a_crs_keeps_the_legacy_estimate(): - minutes, _ = fs.walk_route(disconnected_graph(), (52.0, 13.0), (52.0, 14.0)) +def test_route_time_fallback_without_a_crs_keeps_the_legacy_estimate(): + minutes, _ = fs.route_time(disconnected_graph(), (52.0, 13.0), (52.0, 14.0)) assert minutes == pytest.approx(111_000 / 83.33, rel=0.01) -def test_walk_route_uses_the_network_when_a_path_exists(): +def test_route_time_uses_the_network_when_a_path_exists(): G = nx.MultiDiGraph(crs="EPSG:4326") G.add_node(1, x=13.0, y=52.0) G.add_node(2, x=13.01, y=52.0) G.add_edge(1, 2, length=1000.0) - minutes, route = fs.walk_route(G, (52.0, 13.0), (52.0, 13.01), projected_crs=BERLIN_CRS) + minutes, route = fs.route_time(G, (52.0, 13.0), (52.0, 13.01), projected_crs=BERLIN_CRS) assert minutes == pytest.approx(1000.0 / 83.33, rel=1e-6) assert route == [(52.0, 13.0), (52.0, 13.01)] @@ -1200,8 +1200,8 @@ def test_a_precomputed_dest_node_gives_the_identical_route(): G = two_node_graph() orig, dest = (52.0, 13.0), (52.0, 13.01) - without = fs.walk_route(G, orig, dest, projected_crs=BERLIN_CRS) - with_nodes = fs.walk_route(G, orig, dest, projected_crs=BERLIN_CRS, + without = fs.route_time(G, orig, dest, projected_crs=BERLIN_CRS) + with_nodes = fs.route_time(G, orig, dest, projected_crs=BERLIN_CRS, orig_node=fs.nearest_node(G, orig), dest_node=fs.nearest_node(G, dest)) assert with_nodes == without @@ -1212,7 +1212,7 @@ def must_not_be_called(*args, **kwargs): raise AssertionError("nearest_nodes ran despite both endpoints being supplied") monkeypatch.setattr(fs.ox.distance, "nearest_nodes", must_not_be_called) - minutes, _ = fs.walk_route(two_node_graph(), (52.0, 13.0), (52.0, 13.01), + minutes, _ = fs.route_time(two_node_graph(), (52.0, 13.0), (52.0, 13.01), orig_node=1, dest_node=2) assert minutes == pytest.approx(1000.0 / 83.33, rel=1e-6) @@ -1227,7 +1227,7 @@ def counting(G, x, y, **kwargs): return real(G, x, y, **kwargs) monkeypatch.setattr(fs.ox.distance, "nearest_nodes", counting) - fs.walk_route(two_node_graph(), (52.0, 13.0), (52.0, 13.01)) + fs.route_time(two_node_graph(), (52.0, 13.0), (52.0, 13.01)) assert len(calls) == 2 @@ -1256,19 +1256,29 @@ def offline_run(monkeypatch, tmp_path): Callable more than once per test, so two configs can be compared - hence the queue being refilled per run rather than a one-shot iterator. + + `graphs` supplies a specific graph for a mode; anything not named there gets + a fresh `chain_graph()`. Since both modes otherwise get the *same* geometry, + a test that needs to prove a route ran over the right network has to pass its + own - identical graphs can't tell the two apart. """ monkeypatch.setattr(fs, "geocode_safe", lambda addr, label, **kw: CHAIN_COORDS[addr]) responses: list[Any] = [] monkeypatch.setattr(fs, "query_with_retry", lambda fn, **kw: responses.pop(0)) - def run(config: dict) -> pd.DataFrame: + def run(config: dict, graphs: dict[str, nx.MultiDiGraph] | None = None) -> pd.DataFrame: config["output"] = { "csv_file": str(tmp_path / "scores.csv"), "html_file": str(tmp_path / "map.html"), } - # run() makes exactly two Overpass-backed calls: the graph, then the POIs. - responses[:] = [chain_graph(), gpd.GeoDataFrame()] + # run() downloads one graph per travel mode present in `destinations`, in + # first-mentioned order, and then the POIs - so the queue has to match. + modes = list(dict.fromkeys( + fs.destination_mode(info) for info in config.get("destinations", {}).values() + )) + responses[:] = [(graphs or {}).get(mode) or chain_graph() for mode in modes] + responses.append(gpd.GeoDataFrame()) return fs.FlatScorer(config, verbose=False).run() return run @@ -1322,7 +1332,7 @@ def one_destination_config(**parameters) -> dict: def test_the_configured_walking_speed_reaches_the_commute_times(offline_run): - """The whole point: tunable from config, without editing walk_route's default.""" + """The whole point: tunable from config, without editing route_time's default.""" df = offline_run(one_destination_config(walking_speed_m_per_min=100.0)) assert df.iloc[0]["near_office_walk_min"] == pytest.approx(10.0, abs=0.05) @@ -1349,13 +1359,223 @@ def test_a_slower_pace_moves_the_score_not_just_the_reported_minutes(offline_run assert slow.iloc[0]["score"] < brisk.iloc[0]["score"] -def test_walk_routes_default_is_the_documented_constant(): +def test_route_times_default_speed_is_the_documented_constant(): """The default is a standalone-use fallback; run() overrides it either way.""" import inspect - default = inspect.signature(fs.walk_route).parameters["walking_speed_m_per_min"].default + default = inspect.signature(fs.route_time).parameters["speed_m_per_min"].default assert default == fs.DEFAULT_WALKING_SPEED_M_PER_MIN assert fs.DEFAULT_WALKING_SPEED_M_PER_MIN * 60 / 1000 == pytest.approx(5.0, abs=0.01) def test_the_shipped_default_config_carries_a_walking_speed(): assert fs.DEFAULT_CONFIG["parameters"]["walking_speed_m_per_min"] == fs.DEFAULT_WALKING_SPEED_M_PER_MIN + + +# ----------------------------------------------------------------- cycling mode -- + +def test_a_destination_may_declare_either_travel_mode(): + for mode in ("walk", "bike"): + config = valid_config(destinations={"Work": {"address": "2 Office Rd", "weight": 0.2, "mode": mode}}) + assert fs.validate_config(config) == [], mode + + +def test_an_unknown_travel_mode_is_rejected_and_names_the_destination(): + problem = only_problem(valid_config( + destinations={"Work": {"address": "2 Office Rd", "weight": 0.2, "mode": "drive"}})) + assert "destinations['Work']" in problem + assert "'drive'" in problem + assert "bike" in problem and "walk" in problem + + +def test_a_destination_without_a_mode_walks(): + """Every config written before cycling existed is an all-walk config.""" + assert fs.validate_config(valid_config()) == [] + assert fs.destination_mode({"address": "2 Office Rd"}) == "walk" + assert fs.destination_mode({"address": "2 Office Rd", "mode": "bike"}) == "bike" + + +def test_a_non_positive_cycling_speed_is_rejected(): + problem = only_problem(valid_config(parameters={"cycling_speed_m_per_min": 0})) + assert "cycling_speed_m_per_min" in problem + assert "greater than 0" in problem + + +def test_the_shipped_default_config_carries_a_cycling_speed(): + assert fs.DEFAULT_CONFIG["parameters"]["cycling_speed_m_per_min"] == fs.DEFAULT_CYCLING_SPEED_M_PER_MIN + assert fs.DEFAULT_CYCLING_SPEED_M_PER_MIN * 60 / 1000 == pytest.approx(15.0, abs=0.01) + + +def test_a_commute_column_carries_its_mode(): + """A cycling commute must never be reported in a column that says 'walk'.""" + assert fs.commute_column("Near Office") == "near_office_walk_min" + assert fs.commute_column("Near Office", "walk") == "near_office_walk_min" + assert fs.commute_column("Near Office", "bike") == "near_office_bike_min" + + +# The cycling network, laid out so that reusing a *walking* node id on it lands +# somewhere plausible instead of raising: the ids overlap, the places do not. +# node 3 @ 13.000 (the flat) --500m-- node 4 @ 13.010 (Near Office) +# node 1 @ 13.050 --1500m-- node 2 @ 13.030 --1500m-- node 4 +# So the honest bike route is 500 m, while every way of getting the nodes from +# the wrong graph yields 1500, 2000 or 3000 m - wrong, and wrong quietly. +def bike_graph() -> nx.MultiDiGraph: + G = nx.MultiDiGraph(crs="EPSG:4326") + for node, lon in ((1, 13.05), (2, 13.03), (3, 13.0), (4, 13.01)): + G.add_node(node, x=lon, y=52.0) + for a, b, length in ((1, 2, 1500.0), (2, 4, 1500.0), (4, 3, 500.0)): + G.add_edge(a, b, length=length) + G.add_edge(b, a, length=length) + return G + + +def mixed_mode_config(**parameters) -> dict: + """One walked and one cycled destination, both at the same address. + + Same address on purpose: the two commutes can then only differ because they + were routed over different networks, not because they went somewhere else. + """ + return valid_config( + destinations={ + "Near Office": {"address": "2 Office Rd", "weight": 0.2}, + "Bike Office": {"address": "2 Office Rd", "weight": 0.2, "mode": "bike"}, + }, + parameters=parameters, + ) + + +def test_each_mode_resolves_its_nodes_in_its_own_graph(offline_run): + """The per-mode sibling of the per-destination node cache test. + + A node id means nothing outside the graph it came from, so caching one + across modes doesn't raise - it silently answers with another junction's + commute. Both endpoints are covered: on the walk graph the flat is node 1 + and the office node 2, on the bike graph they are 3 and 4. + """ + df = offline_run(mixed_mode_config(cycling_speed_m_per_min=250.0), + graphs={"bike": bike_graph()}) + row = df.iloc[0] + assert row["near_office_walk_min"] == pytest.approx(1000.0 / 83.33, abs=0.05) + assert row["bike_office_bike_min"] == pytest.approx(500.0 / 250.0, abs=0.05) + + +def test_the_configured_cycling_speed_reaches_the_bike_commute_times(offline_run): + df = offline_run(mixed_mode_config(cycling_speed_m_per_min=100.0), + graphs={"bike": bike_graph()}) + assert df.iloc[0]["bike_office_bike_min"] == pytest.approx(5.0, abs=0.05) + + +def test_an_absent_cycling_speed_falls_back_to_the_default(offline_run): + df = offline_run(mixed_mode_config(), graphs={"bike": bike_graph()}) + assert df.iloc[0]["bike_office_bike_min"] == pytest.approx( + 500.0 / fs.DEFAULT_CYCLING_SPEED_M_PER_MIN, abs=0.05) + + +def test_the_walking_pace_does_not_leak_into_a_bike_commute(offline_run): + """Each mode divides by its own pace; sharing one would be invisible.""" + df = offline_run(mixed_mode_config(walking_speed_m_per_min=250.0, cycling_speed_m_per_min=250.0), + graphs={"bike": bike_graph()}) + assert df.iloc[0]["near_office_walk_min"] == pytest.approx(4.0, abs=0.05) + assert df.iloc[0]["bike_office_bike_min"] == pytest.approx(2.0, abs=0.05) + + +def test_a_mixed_config_costs_one_node_lookup_per_candidate_per_mode(monkeypatch, offline_run): + lookups = [] + real = fs.ox.distance.nearest_nodes + + def counting(G, x, y, **kwargs): + lookups.append((x, y)) + return real(G, x, y, **kwargs) + + monkeypatch.setattr(fs.ox.distance, "nearest_nodes", counting) + config = mixed_mode_config() + config["candidates"].append({"name": "Flat B", "address": "3 Far Office Rd", "rent": 1500}) + offline_run(config, graphs={"bike": bike_graph()}) + + # 2 candidates x 2 modes + 2 destinations = 6. Two candidates, not one, so + # the number actually discriminates: resolving a node per leg instead would + # be 2*2*2 = 8, which at one candidate collides with the correct answer. + assert len(lookups) == 6 + + +def test_cycling_a_far_destination_scores_better_than_walking_it(offline_run): + """The reason the feature exists: a 36-minute walk is a 12-minute cycle.""" + far = {"address": "3 Far Office Rd", "weight": 0.5} + walked = offline_run(valid_config(destinations={"Far Office": dict(far)})) + cycled = offline_run(valid_config(destinations={"Far Office": dict(far, mode="bike")})) + + assert walked.iloc[0]["far_office_walk_min"] == pytest.approx(36.0, abs=0.1) + assert cycled.iloc[0]["far_office_bike_min"] == pytest.approx(12.0, abs=0.1) + assert cycled.iloc[0]["score"] > walked.iloc[0]["score"] + + +@pytest.fixture +def run_recording_downloads(monkeypatch, tmp_path): + """Like `offline_run`, but lets the Overpass-backed callables actually run. + + `offline_run` replaces `query_with_retry` with the canned answer, so it never + executes the closure and can't see which `network_type` was asked for. This + one stubs osmnx itself instead, which is the only way to prove the cycling + graph is a *bike* download rather than a second walk one. + """ + requested: list[str] = [] + + def fake_graph_from_bbox(bbox=None, network_type=None, **kwargs): + requested.append(network_type) + return bike_graph() if network_type == "bike" else chain_graph() + + monkeypatch.setattr(fs, "geocode_safe", lambda addr, label, **kw: CHAIN_COORDS[addr]) + monkeypatch.setattr(fs.ox, "graph_from_bbox", fake_graph_from_bbox) + monkeypatch.setattr(fs.ox, "features_from_bbox", lambda **kwargs: gpd.GeoDataFrame()) + monkeypatch.setattr(fs, "query_with_retry", lambda fn, **kw: fn()) + + def run(config: dict) -> tuple[list[str], pd.DataFrame]: + config["output"] = { + "csv_file": str(tmp_path / "scores.csv"), + "html_file": str(tmp_path / "map.html"), + } + return requested, fs.FlatScorer(config, verbose=False).run() + + return run + + +def test_an_all_walk_config_downloads_exactly_one_network(run_recording_downloads): + """The whole point of downloading lazily: existing configs pay nothing extra.""" + requested, _ = run_recording_downloads(valid_config(destinations={ + "Near Office": {"address": "2 Office Rd", "weight": 0.2}, + "Far Office": {"address": "3 Far Office Rd", "weight": 0.2, "mode": "walk"}, + })) + assert requested == ["walk"] + + +def test_a_mixed_config_downloads_one_network_per_mode(run_recording_downloads): + requested, df = run_recording_downloads(mixed_mode_config(cycling_speed_m_per_min=250.0)) + assert requested == ["walk", "bike"] + # And the bike leg really came off the bike graph, not a second walk one. + assert df.iloc[0]["bike_office_bike_min"] == pytest.approx(2.0, abs=0.05) + + +def test_an_all_bike_config_downloads_only_the_bike_network(run_recording_downloads): + requested, _ = run_recording_downloads(valid_config(destinations={ + "Bike Office": {"address": "2 Office Rd", "weight": 0.2, "mode": "bike"}, + })) + assert requested == ["bike"] + + +def test_a_config_with_no_destinations_downloads_no_street_network(run_recording_downloads): + """Nothing to route means nothing to route over - the graph was never used.""" + requested, df = run_recording_downloads(valid_config(destinations={})) + assert requested == [] + assert not [col for col in df.columns if col.endswith(("_walk_min", "_bike_min"))] + + +def test_a_mixed_map_names_its_route_layer_for_both_modes(offline_run, tmp_path): + offline_run(mixed_mode_config(), graphs={"bike": bike_graph()}) + mixed = (tmp_path / "map.html").read_text(encoding="utf-8") + + offline_run(one_destination_config()) + walk_only = (tmp_path / "map.html").read_text(encoding="utf-8") + + # An all-walk map keeps the layer name it has always had. + assert "Predicted walking routes" in walk_only + assert "Predicted commute routes" in mixed + assert "Predicted walking routes" not in mixed