batpad
commented
5 years ago I think this is an interesting issue, with a fundamental issue being that within the current architecture, each "comparator" only receives the old and new version of the single feature modified / being checked. As you point out @vgeorge, for many comparators, it is useful to have some more "surrounding context" to be able to meaningfully make an assertion about the change.
I know this is something that has come up / been discussed before, and any solution is potentially a bit tricky / has implications on the performance of the entire pipeline, since these comparators need to be run for every single edit in OSM, and adding any extra work / async activity in them must be measured very carefully.
Currently, the approach that has been favoured has been a pre-processing step where the data that is sent to the comparators is "augmented" with additional context data that maybe useful in the comparison - for eg. - additional metadata about the user that made the edit. It does seem like it would be extremely useful to be able to add a property like member_of_ways or so to nodes that would be an array of other ways that the node being edited is a part of, allowing one to make more meaningful assertions about the change.
This is a hard problem though :-) - I think the first step might be having an extremely responsive API that one could query to get all ways a node is a part of, and then hook that up to a pre-processing pipeline to lookup nodes against before the change data gets passed to osm-compare.
In general, I think it might be nice to revisit the discussion of "additional context" that can be passed to the comparators - one starts hitting a serious wall on the kinds of assertions one can make about a change without having access to map data of the area immediately surrounding the change. One would ideally probably like to have access to perhaps the GeoJSON of the entire z15 (or so) tile that the feature being edited is on. Any attempt to do this, though, will have many infrastructure implications / would have to be thought through carefully, balancing practicality with the need to get additional data -- perhaps fetching this additional data only for a subset of edits, etc.
This is a super interesting problem though, and I hope we can find a way to collectively think through / work on this a bit as being able to have some of this additional context within a compare function would drastically increase the usefulness of the assertions one can make about certain kinds of edits.
vgeorge
I've been watching an area in OSMCha and received false positives of "impossible angles" comparator. Here is an example:
https://osmcha.mapbox.com/changesets/66565198
As @willemarcel stated at https://github.com/mapbox/osmcha-frontend/issues/168, the comparator do not consider the connecting way and returns the warning.
My suggestion here is to check if the vertex node is part of another way when a narrow angle is detected, and do not return the warning if that is the case.