Skip to content
This repository has been archived by the owner on Mar 9, 2023. It is now read-only.

Split of "#111 Fix lane count calculation" #146

Merged
merged 55 commits into from
Apr 25, 2022

Conversation

droogmic
Copy link
Collaborator

This is what #111 would look like applied to:

This is getting to a reviewable size.

BudgieInWA and others added 30 commits March 25, 2022 17:09
Forward and backward lanes are independently guessed to be half of the lanes, but the tags on the website assume that providing `lanes` and `lanes:backward` is enough.
Co-authored-by: droogmic <droogmic@gmail.com>
Incorporate lanes:both_ways and centre_turn_lanes.
Add assumption that oneway=no, lanes=1 means lanes:both_ways=1
Reveals a bunch of failed tests...
…tructure.

The code is a mess, gesturing generally at the idea, without compiling. I feel like
the whole lot should mean close to what it actually says.
This is much closer to something useful. Still doesn't compile.
 Conflicts:
	rust/osm2lanes/src/transform/lanes_to_tags.rs
	rust/osm2lanes/src/transform/tags_to_lanes/bus.rs
	rust/osm2lanes/src/transform/tags_to_lanes/mod.rs
each schema parses their own tags, and so we always would want to warn.
@@ -255,6 +296,7 @@ fn lanes_bus(
Ok(())
}

//noinspection ALL
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops, this was a side-effect of trying to fix that reported error. Well spotted.

@droogmic
Copy link
Collaborator Author

I think this is getting to a reasonable size, if you approve this I will chain merge #141, #145 and finally this one.

@droogmic droogmic changed the base branch from rust_guess_lane_count_target to main April 25, 2022 16:06
@droogmic droogmic merged commit b387937 into a-b-street:main Apr 25, 2022
@droogmic droogmic deleted the rust_guess_lane_count_source branch April 25, 2022 16:26
Comment on lines 215 to 228
// Parse lane count schemas first.
let oneway = Oneway::from(
tags.is_any("oneway", &["yes", "-1"]) || tags.is("junction", "roundabout"),
);
let bus_lane_counts = BusLanesCount::from_tags(tags, locale, oneway, warnings)?;
let centre_turn_lanes = CentreTurnLaneScheme::new(tags, oneway, locale, warnings);
let lanes = Counts::new(
tags,
oneway,
&centre_turn_lanes,
&bus_lane_counts,
locale,
warnings,
);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love the idea that the road builder is responsible for all of this. It's pretty clearly the right spot.

Combined with "builder" in the name, it makes me think of something like this. The call graph becomes the things that describes the dependencies, which is interesting.

struct RoadBuilder {
    // ...
    pub oneway: Oneway,
    pub bus_lane_counts: BusLanesCount,
    pub centre_turn_lanes: CentreTurnLaneScheme,
    pub lane_counts: Counts,

    pub locale: Locale,
    warnings: RoadWarnings,
}

impl  RoadBuilder {
	pub fn from_tags(tags: &Tags, locale: &Locale) -> Result {
	      let road = Self::local_default(locale);
	      road.oneway = Oneway::from(tags.is_any("oneway", &["yes", "-1"]) || tags.is("junction", "roundabout"));
	      road.bus_lane_counts = BusLanesCount::from_tags(tags, &self)?;
	      road.centre_turn_lanes = CentreTurnLaneScheme::from_tags(tags, &self)?;
	      road.lane_counts = Counts::from_tags(tags, &self);
	}
	pub fn add_warning(...){...}
}

// ::default() for everything...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants