Skip to content

Commit

Permalink
only comparator if function.length === 2 (#238)
Browse files Browse the repository at this point in the history
* only comparator if function.length === 2

* only comparator if function.length === 2

* only comparator if function.length === 2

* only comparator if function.length === 2

* import ascending

* relax error message test

* Update README
  • Loading branch information
mbostock authored Oct 3, 2021
1 parent 81f2c7c commit e6501c9
Show file tree
Hide file tree
Showing 6 changed files with 13 additions and 11 deletions.
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -543,7 +543,7 @@ For descending order, negate the group value:
d3.groupSort(barley, g => -d3.median(g, d => d.yield), d => d.variety)
```

If a *comparator* is passed instead of an *accessor* (i.e., if the second argument is a function that takes two arguments), it will be asked to compare two groups *a* and *b* and should return a negative value if *a* should be before *b*, a positive value if *a* should be after *b*, or zero for a partial ordering.
If a *comparator* is passed instead of an *accessor* (i.e., if the second argument is a function that takes exactly two arguments), it will be asked to compare two groups *a* and *b* and should return a negative value if *a* should be before *b*, a positive value if *a* should be after *b*, or zero for a partial ordering.

<a name="count" href="#count">#</a> d3.<b>count</b>(<i>iterable</i>[, <i>accessor</i>]) · [Source](https://github.com/d3/d3-array/blob/main/src/count.js), [Examples](https://observablehq.com/@d3/d3-count)

Expand Down Expand Up @@ -744,7 +744,7 @@ Returns an array containing the values in the given *iterable* in the sorted ord
d3.sort(new Set([0, 2, 3, 1])) // [0, 1, 2, 3]
```

If an *accessor* (a function that takes a single argument) is specified,
If an *accessor* (a function that does not take exactly two arguments) is specified,

```js
d3.sort(data, d => d.value)
Expand Down
2 changes: 1 addition & 1 deletion src/bisector.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ export default function bisector(f) {
let compare1 = f;
let compare2 = f;

if (f.length === 1) {
if (f.length !== 2) {
delta = (d, x) => f(d) - x;
compare1 = ascending;
compare2 = (d, x) => ascending(f(d), x);
Expand Down
2 changes: 1 addition & 1 deletion src/groupSort.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import group, {rollup} from "./group.js";
import sort from "./sort.js";

export default function groupSort(values, reduce, key) {
return (reduce.length === 1
return (reduce.length !== 2
? sort(rollup(values, reduce, key), (([ak, av], [bk, bv]) => ascending(av, bv) || ascending(ak, bk)))
: sort(group(values, key), (([ak, av], [bk, bv]) => reduce(av, bv) || ascending(ak, bk))))
.map(([key]) => key);
Expand Down
2 changes: 1 addition & 1 deletion src/rank.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ export default function rank(values, valueof = ascending) {
if (typeof values[Symbol.iterator] !== "function") throw new TypeError("values is not iterable");
let V = Array.from(values);
const R = new Float64Array(V.length);
if (valueof.length === 1) V = V.map(valueof), valueof = ascending;
if (valueof.length !== 2) V = V.map(valueof), valueof = ascending;
const compareIndex = (i, j) => valueof(V[i], V[j]);
let k, r;
Uint32Array
Expand Down
8 changes: 5 additions & 3 deletions src/sort.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import ascending from "./ascending.js";
import permute from "./permute.js";

export default function sort(values, ...F) {
if (typeof values[Symbol.iterator] !== "function") throw new TypeError("values is not iterable");
values = Array.from(values);
let [f] = F;
if ((f && f.length === 1) || F.length > 1) {
if ((f && f.length !== 2) || F.length > 1) {
const index = Uint32Array.from(values, (d, i) => i);
if (F.length > 1) {
F = F.map(f => values.map(f));
Expand All @@ -20,10 +21,11 @@ export default function sort(values, ...F) {
}
return permute(values, index);
}
return values.sort(f === undefined ? ascendingDefined : compareDefined(f));
return values.sort(compareDefined(f));
}

export function compareDefined(compare) {
export function compareDefined(compare = ascending) {
if (compare === ascending) return ascendingDefined;
if (typeof compare !== "function") throw new TypeError("compare is not a function");
return (a, b) => {
const x = compare(a, b);
Expand Down
6 changes: 3 additions & 3 deletions test/sort-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,12 @@ it("sort(values) accepts an iterable", () => {
});

it("sort(values) enforces that values is iterable", () => {
assert.throws(() => sort({}), {name: "TypeError", message: "values is not iterable"});
assert.throws(() => sort({}), {name: "TypeError", message: /is not iterable/});
});

it("sort(values, comparator) enforces that comparator is a function", () => {
assert.throws(() => sort([], {}), {name: "TypeError", message: "compare is not a function"});
assert.throws(() => sort([], null), {name: "TypeError", message: "compare is not a function"});
assert.throws(() => sort([], {}), {name: "TypeError", message: /is not a function/});
assert.throws(() => sort([], null), {name: "TypeError", message: /is not a function/});
});

it("sort(values) does not skip sparse elements", () => {
Expand Down

0 comments on commit e6501c9

Please sign in to comment.