Skip to content

fix(clusters-dbscan): honor non-kilometer units in neighbor distance check#3073

Open
monad98 wants to merge 1 commit into
Turfjs:masterfrom
monad98:fix/clusters-dbscan-maxdistance-units
Open

fix(clusters-dbscan): honor non-kilometer units in neighbor distance check#3073
monad98 wants to merge 1 commit into
Turfjs:masterfrom
monad98:fix/clusters-dbscan-maxdistance-units

Conversation

@monad98

@monad98 monad98 commented Jun 16, 2026

Copy link
Copy Markdown

Issue

clustersDbscan ignores a non-kilometer units option. The bbox pre-filter honors options.units via lengthToDegrees(maxDistance, options.units), but the final neighbor predicate computes the distance in kilometers and compares it directly against maxDistance:

const distanceInKm = distance(point, neighborPoint, { units: "kilometers" });
return distanceInKm <= maxDistance;

So maxDistance is treated as kilometers in the predicate regardless of options.units. For any unit smaller than a kilometer this makes the check effectively always true, and the neighborhood is no longer clipped to a maxDistance-radius circle — it collapses to the square bounding box, clustering points that are farther apart than maxDistance.

Example: with { units: "meters" } and maxDistance: 400, two points ~566 m apart (400 m east + 400 m north) are clustered even though they are well beyond 400 m.

Fix

Measure the neighbor distance in options.units so it matches the unit used by both maxDistance and the bbox pre-filter. When units is omitted, both still default to kilometers, so existing behavior is unchanged.

Tests

Added a regression test covering a non-kilometer unit. All existing fixture tests still pass.

…check

The neighbor distance was computed in kilometers and compared directly
against maxDistance, while the bbox pre-filter used options.units. With a
non-kilometer unit (e.g. meters) the comparison was effectively always
true, so the neighborhood collapsed to the square bounding box instead of
a maxDistance-radius circle. Measure the neighbor distance in options.units
so both checks agree.
@mfedderly mfedderly added this to the v8 milestone Jun 18, 2026
@mfedderly

Copy link
Copy Markdown
Collaborator

Oof, we definitely imply that it should work this way in the docs, but I'm afraid to merge this change without marking it as a breaking change. I've got a bunch of prep work for v8 staged, but haven't merged any of it quite yet. I added this to the v8 milestone though so that I should be able to include it there.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants