Generate documentation for OSM tag mappers by leonardehrenfried · Pull Request #5929 · opentripplanner/OpenTripPlanner

leonardehrenfried · 2024-06-26T11:16:17Z

Summary

This adds a doc generator for the OSM tag mappers. This documentation is useful for non-developers to look up which OSM tags influence the routing in what way.

It looks like this:

A few small refactorings are also part of this PR:

A few mapping rules that are very specific to the Portland area are moved to PortlandMapper
The Condition EqualsAnyIn was renamed to OneOf
The old bicycle safety report was removed from the Report API

Issue

Closes #5556

Documentation

✔️

cc @tsobuskerudbyen

codecov · 2024-06-26T11:28:25Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.62%. Comparing base (e1c02b2) to head (07bdb99).
Report is 137 commits behind head on dev-2.x.

Additional details and impacted files

@@              Coverage Diff              @@
##             dev-2.x    #5929      +/-   ##
=============================================
+ Coverage      69.43%   69.62%   +0.19%     
- Complexity     17067    17128      +61     
=============================================
  Files           1935     1937       +2     
  Lines          73612    73740     +128     
  Branches        7540     7547       +7     
=============================================
+ Hits           51115    51345     +230     
+ Misses         19873    19757     -116     
- Partials        2624     2638      +14

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

leonardehrenfried · 2024-06-27T13:38:01Z

Sort menu items alphabetically

leonardehrenfried · 2024-06-27T14:35:39Z

@JohanEntur @tsobuskerudbyen Since you're going to be one of the main readers of this new documentation, can you take a look https://github.com/opentripplanner/OpenTripPlanner/blob/669da728640d42923220743277f7c4b78a3a8ed5/docs/osm/Norway.md and check this is helpful for you?

Once this PR is merged, it will become part of the main OTP documentation and use the regular styling, rather than the Github one.

JohanEntur · 2024-06-28T10:43:20Z

@Bjoern-Rapp 👀

JohanEntur · 2024-06-28T11:19:26Z

It's a good overview to bypass reading the code. Still a complex set to grasp, but this is warmly welcomed.

leonardehrenfried · 2024-07-01T08:19:58Z

@JohanEntur Yes, it will not replace a programmer having to answer complicated questions but it will help casual users to figure out the answer to "Does OTP use tag X?".

optionsome · 2024-07-01T11:41:16Z

+  private static Table propTable(WayPropertySet wps) {
+    var propTable = new TableBuilder();
+    propTable.withHeaders("specifier", "permission", "bike safety", "walk safety");
+
+    for (var prop : wps.getWayProperties()) {
+      propTable.addRow(
+        "`%s`".formatted(prop.specifier().toDocString()),
+        "`%s`".formatted(prop.properties().getPermission()),
+        tableValues(prop.properties().bicycleSafety()),
+        tableValues(prop.properties().walkSafety())
+      );
+    }
+    return propTable.build();
+  }
+
+  private static Table mixinTable(WayPropertySet wps) {
+    var propTable = new TableBuilder();
+    propTable.withHeaders("matcher", "bicycle safety", "walk safety");
+
+    for (var prop : wps.getMixins()) {
+      propTable.addRow(
+        "`%s`".formatted(prop.specifier().toDocString()),
+        tableValues(prop.bicycleSafety()),
+        tableValues(prop.walkSafety())
+      );
+    }
+    return propTable.build();
+  }


Should we apply some sort of sorting on these tables? I don't know what is the most logical one.

Do you have a suggestion? Nothing comes to mind that could define a natural order.

We could make it visually pleasing by ordering them by whether they define bike safety, walk safety or both and then use alphabetic order as a tie breaker.

Personally, I would keep the insertion order but not opposed to another one.

Yep lets just stick with the insertion order.

optionsome · 2024-07-01T11:43:21Z

  }

-  record EqualsAnyIn(String key, String... values) implements Condition {
+  record OneOf(String key, String... values) implements Condition {


A bit of javadoc on these records wouldn't hurt as not all of these are instantly obvious.

Added in 22d2123

tsobuskerudbyen · 2024-07-01T13:42:02Z

Looks good, but the comment above is in a way good and should be written out in the top. Suggestion: "If a OpenStreetMap (OSM) tag is not part of the documentation under it is not used by OpenTripPlanner (OTP)." And what does an empty cell under the "bike safety" column man? E.g. for the highway=bridleway line. Neutral? Not allowed?

leonardehrenfried · 2024-07-02T08:00:37Z

And what does an empty cell under the "bike safety" column man? E.g. for the highway=bridleway line. Neutral? Not allowed?

This means that the bike safety is unaffected by this selector. I could have written "1" but that clutters up the table quite a bit.

optionsome · 2024-07-02T21:11:41Z


-  record EqualsAnyInOrAbsent(String key, String... values) implements Condition {
+  /**
+   * Selects a tag which has one of a set of given values or the tag is absent.


This is slightly difficult to read but I couldn't come up with a better way to formulate what this does.

I've tried to improve it here: a9441a7

…ecifier/Condition.java Co-authored-by: Joel Lappalainen <lappalj8@gmail.com>

leonardehrenfried · 2024-07-03T08:22:43Z

@tsobuskerudbyen I have added a bit more information to the document. Check out the current version here: https://github.com/opentripplanner/OpenTripPlanner/blob/93a132ad9d1ca2a8bddc642b4f917437a4ed73ed/docs/osm/Norway.md

tsobuskerudbyen · 2024-07-03T09:05:00Z

Look good. Thanks!

vpaturet

Looks good, just one typo.

vpaturet · 2024-07-10T11:46:54Z

+  /**
+   * Does forward and back have the same value?
+   */
+  public boolean isSymetric() {


typo: symetric --> symmetric

leonardehrenfried · 2024-07-11T11:04:35Z

@JohanEntur @tsobuskerudbyen The deployed version of the Norwegian documentation is here: https://docs.opentripplanner.org/en/dev-2.x/osm/Norway/

From now on it will always be up to date.

leonardehrenfried added 4 commits June 26, 2024 12:03

Document integration with OSM

f00e907

Skip more mappers

e942193

Move Portland specific tags to PortlandMapper

c28736b

Finetune look of tables

669da72

leonardehrenfried added the !Documentation Improve documentation targeting users and/or developers. label Jun 26, 2024

leonardehrenfried requested a review from a team as a code owner June 26, 2024 11:16

Remove bike safety endpoint from Report API

69f9076

Update Javadoc

75af424

leonardehrenfried requested a review from optionsome June 27, 2024 13:39

optionsome reviewed Jul 1, 2024

View reviewed changes

Apply review feedback

22d2123

Apply review feedback

93a132a

leonardehrenfried force-pushed the document-osm branch from 0772903 to 93a132a Compare July 2, 2024 12:42

leonardehrenfried requested a review from optionsome July 2, 2024 14:25

optionsome reviewed Jul 2, 2024

View reviewed changes

leonardehrenfried and others added 2 commits July 3, 2024 10:13

Update src/main/java/org/opentripplanner/openstreetmap/wayproperty/sp…

b057fbc

…ecifier/Condition.java Co-authored-by: Joel Lappalainen <lappalj8@gmail.com>

Improve Javadoc

a9441a7

leonardehrenfried force-pushed the document-osm branch from 5fe4c9c to a9441a7 Compare July 3, 2024 08:17

leonardehrenfried requested a review from optionsome July 3, 2024 08:17

optionsome previously approved these changes Jul 4, 2024

View reviewed changes

leonardehrenfried requested a review from t2gran July 4, 2024 14:29

leonardehrenfried enabled auto-merge July 4, 2024 14:29

leonardehrenfried requested review from vpaturet and removed request for t2gran July 9, 2024 09:07

vpaturet requested changes Jul 10, 2024

View reviewed changes

Fix typo

07bdb99

leonardehrenfried dismissed optionsome’s stale review via 07bdb99 July 10, 2024 12:39

leonardehrenfried requested review from optionsome and vpaturet July 10, 2024 12:40

vpaturet approved these changes Jul 10, 2024

View reviewed changes

optionsome approved these changes Jul 11, 2024

View reviewed changes

leonardehrenfried merged commit 8404481 into opentripplanner:dev-2.x Jul 11, 2024

t2gran pushed a commit that referenced this pull request Jul 11, 2024

Add changelog entry for #5929 [ci skip]

962d128

leonardehrenfried deleted the document-osm branch July 11, 2024 11:03

t2gran added this to the 2.6 (next release) milestone Jul 23, 2024

Uh oh!

Conversation

leonardehrenfried commented Jun 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Issue

Documentation

Uh oh!

codecov Bot commented Jun 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

leonardehrenfried commented Jun 27, 2024

Uh oh!

leonardehrenfried commented Jun 27, 2024

Uh oh!

JohanEntur commented Jun 28, 2024

Uh oh!

JohanEntur commented Jun 28, 2024

Uh oh!

leonardehrenfried commented Jul 1, 2024

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tsobuskerudbyen commented Jul 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

leonardehrenfried commented Jul 2, 2024

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

leonardehrenfried commented Jul 3, 2024

Uh oh!

tsobuskerudbyen commented Jul 3, 2024

Uh oh!

vpaturet left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

leonardehrenfried commented Jul 11, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

leonardehrenfried commented Jun 26, 2024 •

edited

Loading

codecov Bot commented Jun 26, 2024 •

edited

Loading

tsobuskerudbyen commented Jul 1, 2024 •

edited

Loading