Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't require a sorted array for d3.quantile #975

Closed
msonnabaum opened this issue Dec 22, 2012 · 4 comments
Closed

Don't require a sorted array for d3.quantile #975

msonnabaum opened this issue Dec 22, 2012 · 4 comments
Milestone

Comments

@msonnabaum
Copy link

Although the documentation does state that d3.quantile requires a sorted array, it would be nice if it just took care of the sorting to avoid potential confusion.

I ran into this today while passing it an unsorted array, and the only reason I even spotted that I was getting an incorrect value was that I checked the results against R to make sure they were the same.

Since d3.quantile has more or less the same signature as R's quantile, I expected them to behave similarly. The current behavior could result in some subtle bugs considering that d3.quantile doesn't validate it's input, it just returns the wrong result.

I'm happy to send a PR for this, but wanted to check first to see if there is a reason for this requirement that I'm missing.

@mbostock
Copy link
Member

The reason it doesn't do this is that it's slow to sort arrays (or even check that they are sorted already).

@msonnabaum
Copy link
Author

In that case, it seems a bit arbitrary to require sorting for d3.quantile, but not for d3.median.

@mbostock
Copy link
Member

It's not arbitrary because there's only one median of a given array, whereas there are many quantiles. So, if you're only interested in the median, it's reasonable to sort the array for you. Whereas if you want quantiles, you probably want more than one, and so you should sort the array beforehand.

But I agree it's a somewhat confusing distinction. I suppose we could have a d3.quantile.sorted(array) and d3.median.sorted(array) that take sorted arrays as input when you want it to be fast.

@msonnabaum
Copy link
Author

I very much like the idea of d3.quantile.sorted.

And the issue of multiple quantiles does seem to be the best reason not to sort by default, but since you most likely want multiple quantiles (like for a boxplot), perhaps it makes sense to introduce a d3.quantiles function. It could take an unsorted array and an array of quantiles, and then only sort once.

@mbostock mbostock closed this as completed Jan 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants