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

include scale name in error #1298

Merged
merged 1 commit into from
Feb 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions src/scales/ordinal.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,9 @@ export function ScaleOrdinal(key, channels, {type, interval, domain, range, sche
}
}
}
if (unknown === scaleImplicit) throw new Error("implicit unknown is not supported");
if (unknown === scaleImplicit) {
throw new Error(`implicit unknown on ${key} scale is not supported`);
}
return ScaleO(key, scaleOrdinal().unknown(unknown), channels, {...options, type, domain, range, hint});
}

Expand Down Expand Up @@ -98,8 +100,9 @@ function inferDomain(channels, interval, key) {
const [min, max] = extent(values).map(interval.floor, interval);
return interval.range(min, interval.offset(max));
}
if (values.size > 10e3 && registry.get(key) === position)
throw new Error("implicit ordinal position domain has more than 10,000 values");
if (values.size > 10e3 && registry.get(key) === position) {
throw new Error(`implicit ordinal domain of ${key} scale has more than 10,000 values`);
}
return sort(values, ascendingDefined);
}

Expand Down
16 changes: 9 additions & 7 deletions src/scales/quantitative.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import {
quantile,
quantize,
reverse as reverseof,
pairs,
scaleLinear,
scaleLog,
scalePow,
Expand Down Expand Up @@ -225,9 +224,9 @@ export function ScaleThreshold(
reverse
}
) {
const sign = orderof(arrayify(domain)); // preserve descending domain
if (!pairs(domain).every(([a, b]) => isOrdered(a, b, sign)))
throw new Error(`the ${key} scale has a non-monotonic domain`);
domain = arrayify(domain);
const sign = orderof(domain); // preserve descending domain
if (!isOrdered(domain, sign)) throw new Error(`the ${key} scale has a non-monotonic domain`);
if (reverse) range = reverseof(range); // domain ascending, so reverse range
return {
type: "threshold",
Expand All @@ -237,9 +236,12 @@ export function ScaleThreshold(
};
}

function isOrdered(a, b, sign) {
const s = descending(a, b);
return s === 0 || s === sign;
function isOrdered(domain, sign) {
for (let i = 1, n = domain.length, d = domain[0]; i < n; ++i) {
const s = descending(d, (d = domain[i]));
if (s !== 0 && s !== sign) return false;
}
return true;
}

export function ScaleIdentity() {
Expand Down
9 changes: 8 additions & 1 deletion test/scales/scales-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,14 @@ import it from "../jsdom.js";

it("Plot throws an error if an ordinal position scale has a huge inferred domain", () => {
assert.ok(Plot.cellX({length: 10000}, {x: d3.randomLcg(42)}).plot());
assert.throws(() => Plot.cellX({length: 10001}, {x: d3.randomLcg(42)}).plot());
assert.throws(() => Plot.cellX({length: 10001}, {x: d3.randomLcg(42)}).plot(), /implicit ordinal domain of x scale/);
});

it("Plot throws an error if scale.unknown is set to d3.scaleImplicit", () => {
assert.throws(
() => Plot.plot({color: {type: "ordinal", unknown: d3.scaleImplicit}}),
/implicit unknown on color scale/
);
});

it("Plot does not throw an error if an ordinal color scale has a huge inferred domain", () => {
Expand Down