Skip to content

Commit

Permalink
Simplify try_from to parse with anyhow
Browse files Browse the repository at this point in the history
  • Loading branch information
@BudgieInWA
BudgieInWA committed Dec 4, 2022
1 parent 0ea674c commit 04ea83d
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 53 deletions.
102 changes: 51 additions & 51 deletions osm2streets/src/lanes/placement.rs
View file
Original file line number Diff line number Diff line change
@@ -1,18 +1,17 @@
use abstutil::Tags;
use anyhow::Result;

use super::{LtrLaneNum, Placement, RoadPosition};

use Placement::*;
use RoadPosition::*;

impl TryFrom<&str> for RoadPosition {
type Error = ();

impl RoadPosition {
/// Tries to parse a road position from an osm tag value as per the `placement` scheme.
/// See https://wiki.openstreetmap.org/wiki/Proposed_features/placement#Tagging
///
/// The direction is treated as forward, use `reverse()` on the result if the context is backward.
fn try_from(value: &str) -> Result<Self, Self::Error> {
pub fn parse(value: &str) -> Result<Self> {
match value {
"" => Ok(Center),
"separation" => Ok(Separation),
Expand All @@ -23,66 +22,60 @@ impl TryFrom<&str> for RoadPosition {
"left_of" => Ok(LeftOf(LtrLaneNum::Forward(lane))),
"middle_of" => Ok(MiddleOf(LtrLaneNum::Forward(lane))),
"right_of" => Ok(RightOf(LtrLaneNum::Forward(lane))),
_ => Err(()),
_ => Err(anyhow!("unknown lane position specifier: {kind}")),
}
} else {
Err(())
Err(anyhow!("bad lane number: {lane_str}"))
}
} else {
Err(())
Err(anyhow!("unknown placement value: {value}"))
}
}
}
}
}

impl TryFrom<&Tags> for Placement {
type Error = ();

impl Placement {
/// Tries to parse a placement from a set of OSM tags according to the `placement` scheme.
/// See https://wiki.openstreetmap.org/wiki/Proposed_features/placement#Tagging
///
/// Limitations:
/// - Doesn't validate tag combos, just returns the first interpretation it finds.
/// - Doesn't allow `:end` and `:start` to mix `:forward` and `:back`. Maybe it should?
fn try_from(tags: &Tags) -> Result<Self, Self::Error> {
pub fn parse(tags: &Tags) -> Result<Self> {
if let Some(transition_or_pos) = tags.get("placement") {
if transition_or_pos == "transition" {
Ok(Transition)
} else {
Ok(Consistent(RoadPosition::try_from(
transition_or_pos.as_str(),
)?))
Ok(Consistent(RoadPosition::parse(transition_or_pos.as_str())?))
}
} else if tags.has_any(vec!["placement:start", "placement:end"]) {
Ok(Varying(
RoadPosition::try_from(tags.get("placement:start").map_or("", |s| s.as_str()))?,
RoadPosition::try_from(tags.get("placement:end").map_or("", |s| s.as_str()))?,
RoadPosition::parse(tags.get("placement:start").map_or("", |s| s.as_str()))?,
RoadPosition::parse(tags.get("placement:end").map_or("", |s| s.as_str()))?,
))
} else if let Some(pos) = tags.get("placement:forward") {
Ok(Consistent(RoadPosition::try_from(pos.as_str())?))
Ok(Consistent(RoadPosition::parse(pos.as_str())?))
} else if tags.has_any(vec!["placement:forward:start", "placement:forward:end"]) {
Ok(Varying(
RoadPosition::try_from(
RoadPosition::parse(
tags.get("placement:forward:start")
.map_or("", |s| s.as_str()),
)?,
RoadPosition::try_from(
tags.get("placement:forward:end").map_or("", |s| s.as_str()),
)?,
RoadPosition::parse(tags.get("placement:forward:end").map_or("", |s| s.as_str()))?,
))
} else if let Some(backwards_pos) = tags.get("placement:backward") {
Ok(Consistent(
RoadPosition::try_from(backwards_pos.as_str())?.reverse(),
RoadPosition::parse(backwards_pos.as_str())?.reverse(),
))
} else if tags.has_any(vec!["placement:backward:start", "placement:backward:end"]) {
Ok(Varying(
RoadPosition::try_from(
RoadPosition::parse(
tags.get("placement:backward:start")
.map_or("", |s| s.as_str()),
)?
.reverse(),
RoadPosition::try_from(
RoadPosition::parse(
tags.get("placement:backward:end")
.map_or("", |s| s.as_str()),
)?
Expand All @@ -99,72 +92,79 @@ mod tests {
use super::*;
use std::collections::BTreeMap;
use LtrLaneNum::*;
use Placement::*;
use RoadPosition::*;

#[test]
fn road_position_parses() {
assert_eq!(RoadPosition::try_from(""), Ok(Center));
assert_eq!(RoadPosition::try_from("separation"), Ok(Separation));
assert_eq!(RoadPosition::try_from("left_of:1"), Ok(LeftOf(Forward(1))));
assert_eq!(RoadPosition::parse("").unwrap(), Center);
assert_eq!(RoadPosition::parse("separation").unwrap(), Separation);
assert_eq!(
RoadPosition::parse("left_of:1").unwrap(),
LeftOf(Forward(1))
);
assert_eq!(
RoadPosition::try_from("middle_of:1"),
Ok(MiddleOf(Forward(1)))
RoadPosition::parse("middle_of:1").unwrap(),
MiddleOf(Forward(1))
);
assert_eq!(
RoadPosition::try_from("right_of:1"),
Ok(RightOf(Forward(1)))
RoadPosition::parse("right_of:1").unwrap(),
RightOf(Forward(1))
);
}

#[test]
fn placement_parses() {
assert_eq!(
Placement::try_from(&Tags::new(BTreeMap::from([(
Placement::parse(&Tags::new(BTreeMap::from([(
"placement".into(),
"transition".into()
)]))),
Ok(Transition)
)])))
.unwrap(),
Transition
);

assert_eq!(
Placement::try_from(&Tags::new(BTreeMap::from([(
Placement::parse(&Tags::new(BTreeMap::from([(
"placement".into(),
"right_of:1".into()
)]))),
Ok(Consistent(RightOf(Forward(1))))
)])))
.unwrap(),
Consistent(RightOf(Forward(1)))
);

assert_eq!(
Placement::try_from(&Tags::new(BTreeMap::from([(
Placement::parse(&Tags::new(BTreeMap::from([(
"placement:forward".into(),
"right_of:1".into()
)]))),
Ok(Consistent(RightOf(Forward(1))))
)])))
.unwrap(),
Consistent(RightOf(Forward(1)))
);

assert_eq!(
Placement::try_from(&Tags::new(BTreeMap::from([(
Placement::parse(&Tags::new(BTreeMap::from([(
"placement:backward".into(),
"right_of:1".into()
)]))),
Ok(Consistent(RightOf(Backward(1))))
)])))
.unwrap(),
Consistent(RightOf(Backward(1)))
);

assert_eq!(
Placement::try_from(&Tags::new(BTreeMap::from([
Placement::parse(&Tags::new(BTreeMap::from([
("placement:start".into(), "right_of:1".into()),
("placement:end".into(), "right_of:2".into())
]))),
Ok(Varying(RightOf(Forward(1)), RightOf(Forward(2))))
])))
.unwrap(),
Varying(RightOf(Forward(1)), RightOf(Forward(2)))
);

assert_eq!(
Placement::try_from(&Tags::new(BTreeMap::from([
Placement::parse(&Tags::new(BTreeMap::from([
("placement:backward:start".into(), "right_of:1".into()),
("placement:backward:end".into(), "right_of:2".into())
]))),
Ok(Varying(RightOf(Backward(1)), RightOf(Backward(2))))
])))
.unwrap(),
Varying(RightOf(Backward(1)), RightOf(Backward(2)))
);
}
}
6 changes: 4 additions & 2 deletions osm2streets/src/road.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,10 @@ impl Road {
};

// Ignoring errors for now.
let placement =
Placement::try_from(&osm_tags).unwrap_or(Placement::Consistent(RoadPosition::Center));
let placement = Placement::parse(&osm_tags).unwrap_or_else(|e| {
warn!("bad placement value (using default): {e}");
Placement::Consistent(RoadPosition::Center)
});

let mut result = Self {
id,
Expand Down

0 comments on commit 04ea83d

Please sign in to comment.