Modify types of railways that constitute rail_access

[sgoel]

This PR modifies the RailAccessParser to include and exclude the various OSM Railway types that we need to consider for the Remix rail routing product. See https://paper.dropbox.com/doc/Rail-routing-types-and-appearances--CcoTAuntwCvKSa99WV5edFPyAg-K1oQPVSL3s8eXf6SLyyow for more information about what we want to include and exclude.

After this change merges, the value of rail_access in the various custom costing profiles will change to include everything in the Set.

This addresses https://ridewithvia.atlassian.net/browse/REM-8628

A follow up PR will be needed in the monorepo to bust the cache on the curl request made to this repo in the dockerfile

[davidmh]

Since we're now actively forking from upstream, we should look into updating the tests to cover our use-cases, then plug the test suite into the Github actions.

We should merge this as-is, I can start looking into the tests.

[davidmh]

Looking at the tests, are we sure that would be enough to allow the construction paths?

https://github.com/remix/OpenRailRouting/blob/b3e899bd07bcae6ca622e1631109a9aa2de633b9/src/test/java/de/geofabrik/railway_routing/parsers/OSMRailwayClassParserTest.java

[sgoel]

Looking at the tests, are we sure that would be enough to allow the construction paths?

https://github.com/remix/OpenRailRouting/blob/b3e899bd07bcae6ca622e1631109a9aa2de633b9/src/test/java/de/geofabrik/railway_routing/parsers/OSMRailwayClassParserTest.java

I think for our use case, we don't need to worry about this at the moment unless Matthew is able to find a way that is tagged with service=*. See https://wiki.openstreetmap.org/wiki/Key:service#Railways. There's a really nice diagram on that page that describes the service=* options:

I can take a look at the test further, but in the import step, the code path that you linked to gets processed here: https://github.com/remix/OpenRailRouting/blob/master/src/main/java/de/geofabrik/railway_routing/RailImportRegistry.java#L81 and the code path that I have modified gets processed here: https://github.com/remix/OpenRailRouting/blob/master/src/main/java/de/geofabrik/railway_routing/RailImportRegistry.java#L120

[sgoel]

Related to our slack conversation about making the frontend configurable through the yaml file, I think I can refactor this work to again allow better customization through the yaml / json profiles instead of hard coding changes in the codebase.

[davidmh]

The part I worry about is that the construction paths will still be lumped into the other value. But if you've tested that it doesn't that's fine.

[davidmh]

Could you rebase against master to pick up the new setup? You may need to squash your commits into a valid message.

[sgoel]

The part I worry about is that the construction paths will still be lumped into the other value. But if you've tested that it doesn't that's fine.

I think the reason that this is not an issue is that the all_tracks.json profile is only checking for !rail_access to disallow routing:

{
  "distance_influence": 0,
  "priority": [
    { "if": "!rail_access", "multiply_by": "0" }
  ],
  "speed": [
    { "if": "true", "limit_to": "rail_average_speed" }
  ]
}

Because of that, the parser that lumps construction ways in to OTHER has no affect.

[sgoel]

Could you rebase against master to pick up the new setup? You may need to squash your commits into a valid message.

done

[sgoel]

I spoke with @davidmh over tuple. It looks like we need to modify the RailwayClass.java file to mirror the changes in RailAccessParser.java so that we tag ways with more granularity rather than classifying them as RailwayClass.OTHER. This is necessary for styling the ways on the UI with the /mvt route.

[sgoel]

@davidmh let's hold off on merging this until I add a test for my changes

[davidmh]

The commit I pushed should cover that, can you confirm?

[sgoel]

The commit I pushed should cover that, can you confirm?

It does. I'm going to add one for narrow_gauge as well

[sgoel]

@davidmh I think this is ready to merge