Add some OSM POIs and start a POI density metric #212

dabreegster · 2025-03-08T13:10:28Z

Towards #203 and #157.

Move the 3 existing POI layers from Scotland-wide GJ files just for display into per study area context data
Add two more categories, filled out as we parse the OSM data
Start a prioritization metric for POI density

The density scale likely needs tuning like stats19, though Edinburgh looks OK:

The raw POIs:

dabreegster

Right before merging, I'll update the Cloudflare context data files

dabreegster · 2025-03-08T13:11:44Z

backend/src/boundary_stats.rs

+}
+
+#[derive(Clone, Copy, Debug, Deserialize, Serialize)]
+pub enum POIKind {


I'll work with Cici to refine these categories. We may want more. Maybe GPs and hospitals should be combined. The only purpose of distinguishing is for the layer, not the metric.

dabreegster · 2025-03-08T13:13:10Z

backend/src/create.rs

@@ -481,3 +496,23 @@ fn nodes_to_edges(graph: &Graph, nodes: Vec<utils::osm2graph::IntersectionID>) -
    }
    edges
 }
+
+fn get_poi(tags: &Tags, point: Coord) -> Option<POI> {
+    if tags.is_any("shop", vec!["convenience", "grocery", "supermarket"]) {


These seem to cover most, but there might be niche things like fruit stands tagged differently. We can validate with local knowledge and iterate.

dabreegster · 2025-03-08T13:13:34Z

backend/src/create.rs

+        });
+    }
+
+    if tags.is_any("amenity", vec!["community_centre", "library"]) {


I'm taking the liberty of lumping libraries into this category too, but may change after discussing with Cici

dabreegster · 2025-03-08T13:14:34Z

data_prep/scotland/src/bin/generate_prioritization.rs

@@ -84,8 +92,7 @@ struct PopulationZoneInput {
 impl PopulationZoneInput {
    fn read_all_from_file() -> Result<Vec<Self>> {
        let population_zones = geojson::de::deserialize_feature_collection_str_to_vec(
-            &std::fs::read_to_string("tmp/population.geojson")
-                .expect("missing population.geojson - see get_reference_layers.sh"),
+            &fs_err::read_to_string("tmp/population.geojson")?,


This crate generates nice error messages with the filename that's missing

dabreegster · 2025-03-08T13:15:17Z

data_prep/scotland/src/bin/generate_prioritization.rs

+                pois.push(POI {
+                    kind,
+                    point: poi.geometry,
+                    name: poi.name,


Nit: We used to show the number of pupils per school. Removed here because I'm not sure it actually matters. We could form a description string instead of name if it is important.

dabreegster · 2025-03-08T13:15:56Z

web/src/common/colors.ts

@@ -33,6 +33,9 @@ export let areaColorScale = commonQuintileColorScale.toReversed();
 export let stats19Limits = [0, 1.0, 10.0, 50.0, 100.0, 1000.0];
 export let stats19ColorScale = commonQuintileColorScale.toReversed();

+export let poiLimits = [0, 1.0, 10.0, 50.0, 100.0, 1000.0];


Blindly copying stats19. It seems reasonable enough to start.

dabreegster · 2025-03-08T13:18:06Z

I'm on battery so won't re-run benchmarks till later. I don't expect checking for more points-in-polygon should be that much of a problem.

michaelkirk

You got farther with this than I was expecting, great!

I pushed up a commit to your branch that I think is uncontroversial. LMK if you'd prefer I don't do that in the future - it was expedient.

michaelkirk · 2025-03-08T20:42:43Z

backend/src/create.rs

+            &tags,
+            // TODO If a POI is tagged on the building itself, ideally we'd use its centroid. But
+            // an arbitrary point on the boundary is good enough.
+            node_mapping[node_ids.into_iter().next().unwrap()],


Suggested change

node_mapping[node_ids.into_iter().next().unwrap()],

node_mapping[node_ids[0]],

We could explode with less flourish.

michaelkirk · 2025-03-08T20:49:51Z

web/src/App.svelte

          {#if $backend}
+            {#if $currentProjectKey.startsWith("ltn_cnt/")}


This was pre-existing (my bad!), so don't feel like you need to address it in this PR, but I think we should consolidate on using the appFocus == 'cnt' check as it's a little more explicit.

michaelkirk · 2025-03-08T20:55:44Z

web/src/PickNeighbourhoodMode.svelte

@@ -324,6 +332,9 @@
          {:else if selectedPrioritization == "stats19"}
            <b>Pedestrian and cyclist collisions:</b>
            {(props.number_stats19_collisions / props.area_km2).toFixed(1)} / km²
+          {:else if selectedPrioritization == "pois"}
+            <b>Points of interest:</b>


I think "Point of interest / POI" is a good enough name for an internal label / variable name, but I wonder if it's confusing for external people actually using the application.

For some reason I associate POI with historical placards that you'd find on a tourist map, for things like "On this spot in 1922, such and such happened". That could be my own weird bias though.

I don't immediately have a better suggestion. Maybe "amenities"? Maybe you've got something, or maybe sustrans has some advice, or maybe it's fine as is.

"Land use density" was also mentioned... will bring up in the next meeting.

michaelkirk · 2025-03-08T21:01:09Z

web/src/context/POIs.svelte

+    GP: "#beaed4",
+    Hospital: "#fdc086",
+    Grocery: "#386cb0",
+    CommunityCenter: "#ffff99",
  };


I think using the existing linear-legend is fine for now, because it's expedient. But it strikes me as a little off to have different types of places "on a spectrum", and it's simply not enough space to fit all our POI text in a single row:

To me, something like this makes more sense for categorical data:

(not necessarily the icons, though that'd be cute, I primarily mean the multiline layout:

colored dot: School, colored dot: GP, colored dot: Hospital colored dot: Grocery, colored dot: Community Center

I'm totally fine to merge this as-is for now, but I think addressing this will be necessary in the nearish term.

Oh yeah, I actually meant to comment on running out of space. I'll add a TODO in the issue.

contextual layer

…o being bundled in context data

dabreegster · 2025-03-08T21:58:31Z

Thanks for the review, and totally fine to push commits to the branch -- thanks for the better errors.

I'll upload the new files and merge

dabreegster force-pushed the osm_pois branch from c3bcd06 to 8a6576d Compare March 8, 2025 13:12

dabreegster commented Mar 8, 2025

View reviewed changes

dabreegster requested a review from michaelkirk March 8, 2025 13:16

michaelkirk approved these changes Mar 8, 2025

View reviewed changes

dabreegster added 4 commits March 8, 2025 21:53

Find grocery stores and community centers from OSM, expose as a

d5dff52

contextual layer

Move GPs, schools, and hospitals from a Scotland-wide GeoJSON layer t…

e8f2e98

…o being bundled in context data

Start a new metric for POI density

b90a4c5

Fix tests

981f4ff

PR feedback

194bc86

dabreegster force-pushed the osm_pois branch from 3159427 to 194bc86 Compare March 8, 2025 21:59

dabreegster merged commit 95daa8f into main Mar 8, 2025
1 check passed

dabreegster deleted the osm_pois branch March 8, 2025 22:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add some OSM POIs and start a POI density metric #212

Add some OSM POIs and start a POI density metric #212

dabreegster commented Mar 8, 2025

dabreegster left a comment

dabreegster Mar 8, 2025 •

edited

Loading

dabreegster Mar 8, 2025

dabreegster Mar 8, 2025

dabreegster Mar 8, 2025

dabreegster Mar 8, 2025

dabreegster Mar 8, 2025

dabreegster commented Mar 8, 2025

michaelkirk left a comment

michaelkirk Mar 8, 2025

dabreegster Mar 8, 2025

michaelkirk Mar 8, 2025

dabreegster Mar 8, 2025

michaelkirk Mar 8, 2025

dabreegster Mar 8, 2025

michaelkirk Mar 8, 2025

dabreegster Mar 8, 2025

dabreegster commented Mar 8, 2025

	node_mapping[node_ids.into_iter().next().unwrap()],
	node_mapping[node_ids[0]],

		{#if $backend}
		{#if $currentProjectKey.startsWith("ltn_cnt/")}

Add some OSM POIs and start a POI density metric #212

Add some OSM POIs and start a POI density metric #212

Conversation

dabreegster commented Mar 8, 2025

dabreegster left a comment

Choose a reason for hiding this comment

dabreegster Mar 8, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dabreegster commented Mar 8, 2025

michaelkirk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dabreegster commented Mar 8, 2025

dabreegster Mar 8, 2025 •

edited

Loading