This PR fixes an unfortunate issue with the snaps() method in Atlas. The method is intending to return all Edges within a "snap" distance that can be "snapped" to that location, sorted by the distance from the original point. Unfortunately, there is an unintended consequence to using SortedSet-- while sorting happens easily on insertion using this type of collection, it enforces uniqueness for the Set by the same distance function used to sort. Abstractly, this distance condition could already lead to accidental exclusions, but it's made significantly worse by the implementation of the distance comparator for SnappedLocation: it measures the distance from the initial snapped location to the starting Node of the Edge. This means that all Edges from an intersection will have the same Distance, and thus only one Edge from any given intersection will be allowed into the SortedSet.
The fix for this has been to return a List, sorted by the original comparator.
Potential Impact:
Not too much, other than expected data now coming through snaps() 💀
Description:
This PR fixes an unfortunate issue with the
snaps()
method in Atlas. The method is intending to return all Edges within a "snap" distance that can be "snapped" to that location, sorted by the distance from the original point. Unfortunately, there is an unintended consequence to using SortedSet-- while sorting happens easily on insertion using this type of collection, it enforces uniqueness for the Set by the same distance function used to sort. Abstractly, this distance condition could already lead to accidental exclusions, but it's made significantly worse by the implementation of the distance comparator for SnappedLocation: it measures the distance from the initial snapped location to the starting Node of the Edge. This means that all Edges from an intersection will have the same Distance, and thus only one Edge from any given intersection will be allowed into the SortedSet.The fix for this has been to return a List, sorted by the original comparator.
Potential Impact:
Not too much, other than expected data now coming through
snaps()
💀Unit Test Approach:
Added unit test to capture the expected behavior
Test Results:
💯
In doubt: Contributing Guidelines