Skip to content

Conversation

@tordans
Copy link
Contributor

@tordans tordans commented Dec 10, 2025

Following #14 (comment), this PR changes the QA for crossings to include more crossing cases:

  • is_crossing now includes all crossing=*, including "informal"
  • is_explicit_crossing_no (new) handles crossing=no
  • on the QA map, crossing=no are purple circles
  • …they are part of the count of crossings on a junction
  • A junction is "green" when the sum of crossings and crossing=no are the same as edges
  • The UI shows the crossings and crossing=no in the sentence
  • The QA now looks at the nearest crossing along an edge

Screenshots:
green with two crossing=no
Bildschirmfoto 2025-12-10 um 18 23 20
Bildschirmfoto 2025-12-10 um 18 23 17


Please consider this PR more of a proof of concept. I don't know anything about rust and this was LLM guided. Feel free to close it …

tordans and others added 2 commits December 11, 2025 11:25
- `is_crossing` now includes all `crossing=*`, including "informal"
- `is_explicit_crossing_no` (new) handles `crossing=no`
- on the QA map, `crossing=no` are purple circles
- …they are part of the count of crossings on a junction
- A junction is "green" when the sum of crossings and `crossing=no` are the same as edges
- The UI shows the crossings and `crossing=no` in the sentence
Copy link
Collaborator

@dabreegster dabreegster left a comment

Choose a reason for hiding this comment

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

Thank you! These changes look great, I made a few code style tweaks and one logic addition

f.set_property("arms", GeoJson::from(debug_arms));
f.set_property("crossings", GeoJson::from(debug_crossings));
f.set_property("explicit_non_crossings", GeoJson::from(debug_explicit_non_crossings));
f.set_property("crossing_count", crossing_count);
Copy link
Collaborator

Choose a reason for hiding this comment

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

The count properties are redundant with the debug info, bit simpler to not duplicate info

}
if node.is_crossing() {
crossings.insert(*n);
// Stop iterating along this edge when we hit the first crossing
Copy link
Collaborator

Choose a reason for hiding this comment

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

To handle things like
image

{crossingCount} crossing{#if crossingCount !== 1}s{/if}
{#if explicitNonCrossingCount > 0}, {explicitNonCrossingCount} explicit non-crossing{#if explicitNonCrossingCount !== 1}s{/if}{/if}
{crossingCount}
{crossingCount == 1 ? "crossing" : "crossings"}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thought this would make this more legible, but with the autoformatting, it really doesn't 😅

tags?: Record<string, string>;
is_crossing: boolean;
is_explicit_crossing_no?: boolean;
is_explicit_crossing_no: boolean;
Copy link
Collaborator

Choose a reason for hiding this comment

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

The property is always set, so no ?

@dabreegster dabreegster merged commit 12cff56 into a-b-street:main Dec 11, 2025
@tordans tordans deleted the qa-with-crossing-no branch December 11, 2025 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants