sort accessor #183
sort accessor #183
Conversation
mbostock
force-pushed
the
sort-accessor
branch
from
6b2d599 to
7489625
Compare
| d3.sort(data, (a, b) => d3.ascending(a.value, b.value)) | ||
| ``` | ||
|
|
||
| The *accessor* is only invoked once per element, and thus may be nondeterministic. |
There was a problem hiding this comment.
With this we could almost use the initial index: d3.sort(A, function(d) { return -arguments[1]; }) works—the only caveat is we can't write function(d,i) since it would have a length===2.
There was a problem hiding this comment.
Sorry, but I’m not understanding this comment. This:
d3.sort(A, function(d) { return -arguments[1]; })This is an elaborate equivalent to this?
d3.reverse(A)To avoid the length ambiguity we could introduce a separate method, say d3.sortBy, that takes an accessor rather than a comparator. But I think it’s convenient to use the same length test that d3.bisector used. I don’t think you’ll often want to reference the original index in an accessor, and if you did, you could always just do the sorting yourself.
There was a problem hiding this comment.
Yes the example was too simple—what I had in mind was more "sorting with reference to an external array of values". But it was more a comment in passing than a specific concern.
| if (typeof values[Symbol.iterator] !== "function") throw new TypeError("values is not iterable"); | ||
| return Array.from(values).sort(comparator); | ||
| values = Array.from(values); |
There was a problem hiding this comment.
Not exactly related to this commit, but aren't we losing something by using plain arrays?
In many cases (such as the optimal transport) I'm sorting a Float32Array of indices against values that I'm reading somewhere else.
To take a simple example, I want to sort all the pixels of an image based on their red value. I'd have indices = Float32Array.from({length: w * h}, (_,i) => i) and would want to do sorted = d3.sort(indices, i => imageData.data[4*i]). I'd rather have sorted be a Float32Array (and maybe the intermediate f, but I don't think it's generically possible since the accessor might return a string).
There was a problem hiding this comment.
We discussed type preservation of the input array previously in #170 (comment), and I still think it’s appropriate that these array methods always return plain arrays for generality and simplicity. If you want something fancier, you can call array.sort directly.
|
Yes, I expect it might be slower than a simple accessor, but I think it’s more important that it does the right thing even if the accessor is nondeterministic. And the only way to do that is to materialize the values. You can always call array.sort or pass a comparator if you care about performance. |
Fixes #182.