Skip to content

Commit 2cdaf18

Browse files
Filmbostock
andauthored
bail out if the inferred ordinal domain has more than 10k values (#858)
* bail out if the inferred ordinal domain has more than 10k values (see #849 (comment)) * only on point and band scales * only throw an error for position scales * apply review suggestion: key as first argument to ScaleO * shorter error message Co-authored-by: Mike Bostock <[email protected]>
1 parent 8157b58 commit 2cdaf18

File tree

2 files changed

+22
-10
lines changed

2 files changed

+22
-10
lines changed

src/scales/ordinal.js

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import {ascendingDefined} from "../defined.js";
44
import {isNoneish, map} from "../options.js";
55
import {maybeInterval} from "../transforms/interval.js";
66
import {maybeSymbol} from "../symbols.js";
7-
import {registry, color, symbol} from "./index.js";
7+
import {registry, color, position, symbol} from "./index.js";
88
import {maybeBooleanRange, ordinalScheme, quantitativeScheme} from "./schemes.js";
99

1010
// This denotes an implicitly ordinal color scale: the scale type was not set,
@@ -13,7 +13,7 @@ import {maybeBooleanRange, ordinalScheme, quantitativeScheme} from "./schemes.js
1313
// of this by setting the type explicitly.
1414
export const ordinalImplicit = Symbol("ordinal");
1515

16-
export function ScaleO(scale, channels, {
16+
function ScaleO(key, scale, channels, {
1717
type,
1818
interval,
1919
domain,
@@ -22,7 +22,7 @@ export function ScaleO(scale, channels, {
2222
hint
2323
}) {
2424
interval = maybeInterval(interval);
25-
if (domain === undefined) domain = inferDomain(channels, interval);
25+
if (domain === undefined) domain = inferDomain(channels, interval, key);
2626
if (type === "categorical" || type === ordinalImplicit) type = "ordinal"; // shorthand for color schemes
2727
if (reverse) domain = reverseof(domain);
2828
scale.domain(domain);
@@ -44,7 +44,7 @@ export function ScaleOrdinal(key, channels, {
4444
...options
4545
}) {
4646
interval = maybeInterval(interval);
47-
if (domain === undefined) domain = inferDomain(channels, interval);
47+
if (domain === undefined) domain = inferDomain(channels, interval, key);
4848
let hint;
4949
if (registry.get(key) === symbol) {
5050
hint = inferSymbolHint(channels);
@@ -68,7 +68,7 @@ export function ScaleOrdinal(key, channels, {
6868
}
6969
}
7070
if (unknown === scaleImplicit) throw new Error("implicit unknown is not supported");
71-
return ScaleO(scaleOrdinal().unknown(unknown), channels, {...options, type, domain, range, hint});
71+
return ScaleO(key, scaleOrdinal().unknown(unknown), channels, {...options, type, domain, range, hint});
7272
}
7373

7474
export function ScalePoint(key, channels, {
@@ -81,7 +81,8 @@ export function ScalePoint(key, channels, {
8181
.align(align)
8282
.padding(padding),
8383
channels,
84-
options
84+
options,
85+
key
8586
);
8687
}
8788

@@ -98,19 +99,20 @@ export function ScaleBand(key, channels, {
9899
.paddingInner(paddingInner)
99100
.paddingOuter(paddingOuter),
100101
channels,
101-
options
102+
options,
103+
key
102104
);
103105
}
104106

105-
function maybeRound(scale, channels, options) {
107+
function maybeRound(scale, channels, options, key) {
106108
let {round} = options;
107109
if (round !== undefined) scale.round(round = !!round);
108-
scale = ScaleO(scale, channels, options);
110+
scale = ScaleO(key, scale, channels, options);
109111
scale.round = round; // preserve for autoScaleRound
110112
return scale;
111113
}
112114

113-
function inferDomain(channels, interval) {
115+
function inferDomain(channels, interval, key) {
114116
const values = new InternSet();
115117
for (const {value, domain} of channels) {
116118
if (domain !== undefined) return domain(); // see channelDomain
@@ -121,6 +123,7 @@ function inferDomain(channels, interval) {
121123
const [min, max] = extent(values).map(interval.floor, interval);
122124
return interval.range(min, interval.offset(max));
123125
}
126+
if (values.size > 10e3 && registry.get(key) === position) throw new Error("implicit ordinal position domain has more than 10,000 values");
124127
return sort(values, ascendingDefined);
125128
}
126129

test/scales/scales-test.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,15 @@ import * as d3 from "d3";
33
import assert from "assert";
44
import it from "../jsdom.js";
55

6+
it("Plot throws an error if an ordinal position scale has a huge inferred domain", () => {
7+
assert.ok(Plot.cellX({length: 10000}, {x: d3.randomLcg(42)}).plot());
8+
assert.throws(() => Plot.cellX({length: 10001}, {x: d3.randomLcg(42)}).plot());
9+
});
10+
11+
it("Plot does not throw an error if an ordinal color scale has a huge inferred domain", () => {
12+
assert.ok(Plot.dotX({length: 10001}, {x: 0, fill: d3.randomLcg(42)}).plot({color: {type: "ordinal"}}));
13+
});
14+
615
it("Plot.scale(description) returns a standalone scale", () => {
716
const color = Plot.scale({color: {type: "linear"}});
817
scaleEqual(color, {

0 commit comments

Comments
 (0)