census areas import #425
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,95 @@ | ||
| use crate::CensusArea; | ||
| use abstutil::Timer; | ||
|
There was a problem hiding this comment. nit: For consistency, we should still split the imports based on category... std, external crates, other crates in this workspace, the current crate. I guess we don't have any contributors using an IDE that forces this order, so not a huge deal either way. There was a problem hiding this comment. happy to abide |
||
| use geo::algorithm::intersects::Intersects; | ||
| use geojson::GeoJson; | ||
| use map_model::Map; | ||
| use std::convert::TryFrom; | ||
|
|
||
| impl CensusArea { | ||
| pub fn find_data_for_map(map: &Map, timer: &mut Timer) -> Result<Vec<CensusArea>, String> { | ||
| // TODO eventually it'd be nice to lazily download the info needed. For now we expect a | ||
| // prepared geojson file to exist in data/system/<city>/population_areas.geojson | ||
|
There was a problem hiding this comment.
|
||
| // | ||
| // When we implement downloading, importer/src/utils.rs has a download() helper that we | ||
| // could copy here. (And later dedupe, after deciding how this crate will integrate with | ||
| // the importer) | ||
| let path = abstutil::path(format!( | ||
| "system/{}/population_areas.geojson", | ||
| map.get_name().city | ||
| )); | ||
| let bytes = abstutil::slurp_file(&path)?; | ||
| debug!("parsing geojson at path: {}", &path); | ||
|
|
||
| // TODO - can we change to Result<_,Box<dyn std::error::Error>> and avoid all these map_err? | ||
|
There was a problem hiding this comment. Sure. I've bounced back and forth how to deal with error types. I don't think there's anywhere that we actually need to look for different error cases, but many places where we need to stringify an error, so I've tried making most APIs use There was a problem hiding this comment. My instincts would be to change it all to I'd love to be able to write: Have you worked with There was a problem hiding this comment. I should say, I don't have much experience with There was a problem hiding this comment. Woops, missed this thread. https://crates.io/crates/anyhow looks nice, pretty much what we need! I've avoided the Rust error ecosystem because there's so much churn, but this one seems simple and popular. I'd be fine moving everything over to this. |
||
| let str = String::from_utf8(bytes).map_err(|e| e.to_string())?; | ||
| timer.start("parsing geojson"); | ||
| let geojson = str.parse::<GeoJson>().map_err(|e| e.to_string())?; | ||
| timer.stop("parsing geojson"); | ||
| let mut results = vec![]; | ||
| let collection = | ||
| geojson::FeatureCollection::try_from(geojson).map_err(|e| e.to_string())?; | ||
|
|
||
| let map_area = map.get_boundary_polygon(); | ||
| let bounds = map.get_gps_bounds(); | ||
|
|
||
| use geo::algorithm::map_coords::MapCoordsInplace; | ||
| let mut geo_map_area: geo::Polygon<_> = map_area.clone().into(); | ||
| geo_map_area.map_coords_inplace(|c| { | ||
| let projected = geom::Pt2D::new(c.0, c.1).to_gps(bounds); | ||
|
There was a problem hiding this comment. Is there a better way to get the map poly in wgs84? Though maybe it's irrelevant, because the other question I need to answer is: is there a blessed way to convert a poly from wgs84 to map coords? There was a problem hiding this comment.
|
||
| (projected.x(), projected.y()) | ||
| }); | ||
|
|
||
| debug!("collection.features: {}", &collection.features.len()); | ||
| timer.start("converting to `CensusArea`s"); | ||
| for feature in collection.features.into_iter() { | ||
| let population = feature.property("population"); | ||
| let total_population = match population { | ||
| Some(serde_json::Value::Number(n)) => n | ||
|
There was a problem hiding this comment. geojson prop parsing is depressingly verbose. I hope this gets better soon! georust/geojson#157 |
||
| .as_u64() | ||
| .expect(&format!("unexpected total population number: {:?}", n)) | ||
| as usize, | ||
| _ => { | ||
| return Err(format!( | ||
| "unexpected format for 'population': {:?}", | ||
| population | ||
| )); | ||
| } | ||
| }; | ||
|
|
||
| let geometry = feature.geometry.expect("geojson feature missing geometry"); | ||
| let mut multi_poly = | ||
| geo::MultiPolygon::<f64>::try_from(geometry.value).map_err(|e| e.to_string())?; | ||
| let geo_polygon = multi_poly | ||
| .0 | ||
| .pop() | ||
| .expect("multipolygon was unexpectedly empty"); | ||
| if !multi_poly.0.is_empty() { | ||
| // Annoyingly upstream GIS has packaged all these individual polygons into | ||
| // "multipolygon" of length 1 Make sure nothing surprising is | ||
| // happening since we only use the first poly | ||
| error!( | ||
| "unexpectedly had {} extra area polygons", | ||
| multi_poly.0.len() | ||
| ); | ||
| } | ||
|
|
||
| if !geo_polygon.intersects(&geo_map_area) { | ||
| debug!( | ||
| "skipping polygon outside of map area. polygon: {:?}, map_area: {:?}", | ||
| geo_polygon, geo_map_area | ||
| ); | ||
| continue; | ||
| } | ||
|
|
||
| let polygon = geom::Polygon::from(geo_polygon); | ||
| results.push(CensusArea { | ||
| polygon, | ||
| total_population, | ||
| }); | ||
| } | ||
| debug!("built {} CensusAreas within map bounds", results.len()); | ||
| timer.stop("converting to `CensusArea`s"); | ||
|
|
||
| Ok(results) | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in general I think it's a little nicer to patch things here at the top level rather than updating across all the individual crates. It's maybe a little more convenient, but more importantly, it works for transitive deps too, which you can't conveniently edit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1. I always forget about this trick