pr: implementing review suggestions

- shrinking pbf test
- remove pbf test from web client
- remove pbf helper function
- fix bail! issue for closures
- switch match statement to if

experimental/src/io.rs

@@ -11,11 +11,11 @@ use osm2streets::{
 };
 
 use crate::network::RoadNetwork;
-use crate::road_functions::{ControlType, Intersection, RoadWay};
 use crate::road_functions::IntersectionType;
+use crate::road_functions::{ControlType, Intersection, RoadWay};
 use crate::road_parts::{Carriage, Designation, RoadEdge, RoadPart};
-use crate::units::{Direction, DrivingSide, Meters, Side, TrafficDirections};
 use crate::units::preamble::*;
+use crate::units::{Direction, DrivingSide, Meters, Side, TrafficDirections};
 
 /// ```
 /// use abstutil::Timer;

osm2streets-js/src/lib.rs

@@ -2,12 +2,12 @@ use std::collections::{BTreeMap, BTreeSet};
 use std::sync::Once;
 
 use abstutil::{Tags, Timer};
-use geom::{Distance, LonLat, Polygon, PolyLine};
+use geom::{Distance, LonLat, PolyLine, Polygon};
 use serde::{Deserialize, Serialize};
 use wasm_bindgen::prelude::*;
 
 use osm2streets::{
-    DebugStreets, Filter, IntersectionID, LaneID, MapConfig, osm, Placement, RoadID, Sidepath,
+    osm, DebugStreets, Filter, IntersectionID, LaneID, MapConfig, Placement, RoadID, Sidepath,
     StreetNetwork, Transformation,
 };
 
@@ -212,7 +212,7 @@ impl JsStreetNetwork {
                     top_pt,
                     top_pt.project_away(width / 2.0, angle.rotate_degs(-135.0)),
                 ])
-                    .make_polygons(width * 0.2)
+                .make_polygons(width * 0.2)
             })
             .collect::<Vec<Polygon>>();
 

streets_reader/src/lib.rs

@@ -54,8 +54,14 @@ pub fn pbf_to_street_network(
     // happens in split_ways.
     streets.config = cfg;
 
-    let (extract, doc) = extract_pfb(&mut streets, input, clip_pts, timer)?;
-    split_ways::split_up_roads(&mut streets, extract, timer);
+    let mut doc = Document::read_pbf(
+        input,
+        clip_pts.as_ref().map(|pts| GPSBounds::from(pts.clone())),
+        timer,
+    )?;
+
+    let out = process_data(&mut streets, clip_pts, timer, &mut doc)?;
+    split_ways::split_up_roads(&mut streets, out, timer);
 
     // Cul-de-sacs aren't supported yet.
     streets.retain_roads(|r| r.src_i != r.dst_i);
@@ -86,36 +92,17 @@ fn extract_osm(
     clip_pts: Option<Vec<LonLat>>,
     timer: &mut Timer,
 ) -> Result<(OsmExtract, Document)> {
-    let mut doc =
-        Document::read(
-            osm_xml_input,
-            clip_pts.as_ref().map(|pts| GPSBounds::from(pts.clone())),
-            timer,
-        )?;
+    let mut doc = Document::read(
+        osm_xml_input,
+        clip_pts.as_ref().map(|pts| GPSBounds::from(pts.clone())),
+        timer,
+    )?;
     // If GPSBounds aren't provided above, they'll be computed in the Document
     let out = process_data(streets, clip_pts, timer, &mut doc)?;
 
     Ok((out, doc))
 }
 
-fn extract_pfb(
-    streets: &mut StreetNetwork,
-    input: &[u8],
-    clip_pts: Option<Vec<LonLat>>,
-    timer: &mut Timer,
-) -> Result<(OsmExtract, Document)> {
-    let mut doc =
-        Document::read_pbf(
-            input,
-            clip_pts.as_ref().map(|pts| GPSBounds::from(pts.clone())),
-            timer,
-        )?;
-
-    let out = process_data(streets, clip_pts, timer, &mut doc)?;
-
-    Ok((out, doc))
-}
-
 fn process_data(
     streets: &mut StreetNetwork,
     clip_pts: Option<Vec<LonLat>>,

streets_reader/src/osm_reader/reader.rs

@@ -53,26 +53,26 @@ impl Document {
                             max_lat: bbox.bottom,
                         });
                     } else if doc.gps_bounds.is_none() {
-                        doc.gps_bounds = Option::from(
-                            scrape_bounds_pbf(
-                                IndexedReader::new(
-                                    Cursor::new(
-                                        input)).unwrap()))
+                        doc.gps_bounds = Option::from(scrape_bounds_pbf(
+                            IndexedReader::new(Cursor::new(input)).unwrap(),
+                        ))
                     }
                 }
                 BlobDecode::OsmData(block) => {
                     block.elements().for_each(|element| {
                         match element {
                             PbfElement::Node(node) => {
-                                let pt = LonLat::new(node.lon(), node.lat()).to_pt(&doc.gps_bounds.clone().unwrap());
+                                let pt = LonLat::new(node.lon(), node.lat())
+                                    .to_pt(&doc.gps_bounds.clone().unwrap());
                                 let mut tags = Tags::new(BTreeMap::new());
                                 for (k, v) in node.tags() {
                                     tags.insert(k, v);
                                 }
                                 doc.nodes.insert(NodeID(node.id()), Node { pt, tags });
                             }
                             PbfElement::DenseNode(node) => {
-                                let pt = LonLat::new(node.lon(), node.lat()).to_pt(&doc.gps_bounds.clone().unwrap());
+                                let pt = LonLat::new(node.lon(), node.lat())
+                                    .to_pt(&doc.gps_bounds.clone().unwrap());
 
                                 let mut tags = Tags::new(BTreeMap::new());
                                 for (k, v) in node.tags() {
@@ -97,10 +97,7 @@ impl Document {
                                         pts.push(node.pt);
                                     }
                                 }
-                                let version = way
-                                    .info()
-                                    .version()
-                                    .map(|x| x as usize);
+                                let version = way.info().version().map(|x| x as usize);
 
                                 if !nodes.is_empty() {
                                     doc.ways.insert(
@@ -122,9 +119,9 @@ impl Document {
                                 let id = RelationID(relation.id());
                                 if doc.relations.contains_key(&id) {
                                     error!("Duplicate IDs detected. Your PBF is corrupt.");
-                                    return
+                                    return;
                                 }
-                                    let mut members = Vec::new();
+                                let mut members = Vec::new();
                                 for member in relation.members() {
                                     let osm_id = match member.member_type {
                                         RelMemberType::Node => {
@@ -190,7 +187,7 @@ impl Document {
         let mut reader = ElementReader {
             tokenizer: xmlparser::Tokenizer::from(raw_string),
         }
-            .peekable();
+        .peekable();
 
         timer.start("scrape objects");
         while let Some(obj) = reader.next() {
@@ -228,7 +225,7 @@ impl Document {
                         obj.attribute("lon").parse::<f64>().unwrap(),
                         obj.attribute("lat").parse::<f64>().unwrap(),
                     )
-                        .to_pt(doc.gps_bounds.as_ref().unwrap());
+                    .to_pt(doc.gps_bounds.as_ref().unwrap());
                     let tags = read_tags(&mut reader);
                     doc.nodes.insert(id, Node { pt, tags });
                 }
@@ -353,23 +350,17 @@ fn scrape_bounds(raw_string: &str) -> GPSBounds {
 
 fn scrape_bounds_pbf(mut reader: IndexedReader<Cursor<&[u8]>>) -> GPSBounds {
     let mut b = GPSBounds::new();
-    reader.for_each_node(|el| {
-        match el {
+    reader
+        .for_each_node(|el| match el {
             PbfElement::Node(node) => {
-                b.update(LonLat::new(
-                    node.lon(),
-                    node.lat(),
-                ));
+                b.update(LonLat::new(node.lon(), node.lat()));
             }
             PbfElement::DenseNode(node) => {
-                b.update(LonLat::new(
-                    node.lon(),
-                    node.lat(),
-                ));
+                b.update(LonLat::new(node.lon(), node.lat()));
             }
             _ => {}
-        }
-    }).expect("Failed to scrape bounds from nodes.");
+        })
+        .expect("Failed to scrape bounds from nodes.");
     b
 }
 

tests/src/frederiksted/test.json

@@ -0,0 +1,5 @@
+{
+	"notes": [
+		"Frederiksted, US Virgin Islands. Exists to test .pbf inputs."
+	]
+}

tests/src/lib.rs

@@ -1,15 +1,13 @@
 #[cfg(test)]
 mod tests {
     use std::path::Path;
-
     use std::sync::Once;
 
+    use abstutil::Timer;
     use anyhow::{bail, Result};
-    use geom::LonLat;
-
     use env_logger::{Builder, Env};
+    use geom::LonLat;
 
-    use abstutil::Timer;
     use experimental::RoadNetwork;
     use osm2streets::{MapConfig, Transformation};
 
@@ -39,23 +37,20 @@ mod tests {
             None
         };
 
-        let (mut street_network, _) = match Path::new(format!("{path}/input.osm").as_str()).exists() {
-            // If XML is found, use it...
-            true => {
-                streets_reader::osm_to_street_network(
-                    &std::fs::read_to_string(format!("{path}/input.osm"))?,
-                    clip_pts,
-                    MapConfig::default(),
-                    &mut timer)?
-            }
-            // ... Otherwise, look for the .osm.pbf
-            false => {
-                streets_reader::pbf_to_street_network(
-                    &std::fs::read(format!("{path}/input.osm.pbf").as_str())?,
-                    clip_pts,
-                    MapConfig::default(),
-                    &mut timer)?
-            }
+        let (mut street_network, _) = if Path::new(format!("{path}/input.osm").as_str()).exists() {
+            streets_reader::osm_to_street_network(
+                &std::fs::read_to_string(format!("{path}/input.osm"))?,
+                clip_pts,
+                MapConfig::default(),
+                &mut timer,
+            )?
+        } else {
+            streets_reader::pbf_to_street_network(
+                &std::fs::read(format!("{path}/input.osm.pbf").as_str())?,
+                clip_pts,
+                MapConfig::default(),
+                &mut timer,
+            )?
         };
         street_network.check_invariants();
         street_network.apply_transformations_with_invariant_checks(
@@ -81,7 +76,7 @@ mod tests {
                 "./{}/geometry.json is different! If it is OK, commit it. \
                 ./{0}/geometry.orig.json is previous result. Compare it on https://geojson.io",
                 path
-                );
+            );
         }
         Ok(())
     }

web/public/tests.json

@@ -25,5 +25,4 @@
   "tempe_light_rail",
   "tempe_split",
   "tiny_loop",
-  "tiny_roundabout",
-  "virgin_islands"]
+  "tiny_roundabout"]