Skip to content

bisector.center - #156

Merged
mbostock merged 1 commit into
masterfrom
bisector-center-138
Aug 23, 2020
Merged

mbostock merged 1 commit into
masterfrom
bisector-center-138

Conversation

@Fil

@Fil Fil commented Jun 24, 2020

Copy link
Copy Markdown
Member

fixes #138

@Fil Fil mentioned this pull request Jun 24, 2020
@Fil

Fil commented Jun 24, 2020

Copy link
Copy Markdown
Member Author

And the demo https://observablehq.com/d/8581f0e7e07ee62b

@Fil
Fil requested a review from mbostock June 25, 2020 08:25
@Fil Fil added the feature label Jul 10, 2020
@mbostock
mbostock force-pushed the bisector-center-138 branch from 799fddd to 107631b Compare August 23, 2020 16:23
@mbostock

Copy link
Copy Markdown
Member

It’s clever how this uses the comparator for most of the work and only relies on the signed distance calculation at the end to choose between the two possible results, such that if an accessor is specified and the accessor doesn’t return a number, the result is NaN and this is equivalent to bisector.left.

There’s a bug here with arrays of size zero and one. I’ll add a fix.

@mbostock

Copy link
Copy Markdown
Member

Oh, and also this needs to the support the optional lo and hi arguments.

@mbostock
mbostock force-pushed the bisector-center-138 branch from 107631b to 8aa9a13 Compare August 23, 2020 16:59
@mbostock

Copy link
Copy Markdown
Member

I found it surprising that bisector.center returns values rather than indexes, unlike bisector.left and bisector.right. So I’ve also rewritten the code to return indexes. And I noticed that it wasn’t consistent with bisector.left in the ordinal case, so I’ve also added some more tests.

@mbostock
mbostock force-pushed the bisector-center-138 branch from 8aa9a13 to 5b78836 Compare August 23, 2020 17:01
@mbostock
mbostock merged commit 5b78836 into master Aug 23, 2020
@mbostock
mbostock deleted the bisector-center-138 branch August 23, 2020 23:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

d3.bisectCenter?

2 participants