Consider inconsistent mapping of rivers as polygons

[cforgaci]

This is an issue raised as part of ropensci/software-review#718

I have one general concern about robustness when deriving river geometry from OSM. Not all rivers are consistently mapped as polygons, and some urban extents include reservoirs/lakes or complex water bodies that may intersect the river centerline but are not the intended “river surface” for corridor delineation. In those cases, the package should ideally (a) document its assumptions about OSM tagging, (b) warn clearly when assumptions are not met, and (c) have a defensible fallback.

[cforgaci]

@fnattino suggests that maybe the solution is to explain better what is happening: documenting assumptions.

[cforgaci]

@yehanwu, I am sharing a few ideas here to kick off the work on this. Feel free to reject, adjust, or add.

River surface data from OSM is only used for riverspace delineation. In that case, the assumption is that any waterbody that intersects the river centerline, including lakes, is part of the river and that viewpoints should always be calculated from water edges, including the edges of those water bodies. Indeed, in many cases, the water polygons might be missing or partial, so the user should be warned that the river centerline was used as a fallback source of viewpoints.

• Document the assumption in function documentation and in riverspace delineation vignette
• Add warning in combine_river_features() if water surface is considered to be absent, e.g., by checking that after clipping (lines 310-311), the river centerline contains linestrings that are longer than, say, 100m (to be checked if this makes sense empirically) representing the assumed width of a possible crossing separating two consecutive water bodies.

For corridor delineation, the assumption is that the spatial (street and rail) network used for delineation is outside the valley and thus any water body intersecting the river centerline, which is used as input instead.

• Document this assumption in the corridor or valley delineation vignette.

[yehanwu]

Add warning in combine_river_features() if water surface is considered to be absent, e.g., by checking that after clipping (lines 310-311), the river centerline contains linestrings that are longer than, say, 100m (to be checked if this makes sense empirically) representing the assumed width of a possible crossing separating two consecutive water bodies.

@cforgaci I was testing some examples for the clipped centerlines (in red), sorted by decreasing length.

From the examples, some longer clipped linestrings are on the edge of the river, for example Poznan: 100.9m (northern end);

Bucharest: 4648m and 1203m.

In Bucharest (zoom in), the 197m and 128m segments seem more like actual gaps between water surface polygons.

Other examples:

So I think we can either:

1. Use 100m as a simple threshold for now. It filters out very small geometry issues, like London (31m, 11m), while still catching possible gaps like Bucharest (197m, 128m) and Senica (larger cases, 8421m). But this can also include many edge cases, like in Poznan and Bucharest.

2. Do not set a threshold for now. Just warn/message that OSM water surface data may be incomplete, and some viewpoints may be calculated from the river centerline instead.

What do you think? I created a PR and will work on the changes after getting your opinions.

[cforgaci]

Thanks @yehan! I would go for option 1 as it filters out some cases like London, and try to provide an informative warning stating that for an n number of river centerline segments of at least 100 m length there is no OSM river surface polygon available; this can be either because the river is underground or lack of data; viewpoints will be calculated from the centerline... of course, phrased better than what I wrote here.