Skip to content
56 changes: 45 additions & 11 deletions src/channel.js
Original file line number Diff line number Diff line change
@@ -1,37 +1,71 @@
import {ascending, descending, rollup, sort} from "d3";
import {first, isIterable, labelof, map, maybeValue, range, valueof} from "./options.js";
import {first, isColor, isEvery, isIterable, labelof, map, maybeValue, range, valueof} from "./options.js";
import {registry} from "./scales/index.js";
import {isSymbol, maybeSymbol} from "./symbols.js";
import {maybeReduce} from "./transforms/group.js";

// TODO Type coercion?
export function Channel(data, {scale, type, value, filter, hint}) {
return {
export function Channel(data, {scale, type, value, filter, hint}, name) {
return inferChannelScale(name, {
scale,
type,
value: valueof(data, value),
label: labelof(value),
filter,
hint
};
});
}

export function Channels(descriptors, data) {
return Object.fromEntries(Object.entries(descriptors).map(([name, channel]) => [name, Channel(data, channel)]));
export function Channels(channels, data) {
return Object.fromEntries(Object.entries(channels).map(([name, channel]) => [name, Channel(data, channel, name)]));
}

// TODO Use Float64Array for scales with numeric ranges, e.g. position?
export function valueObject(channels, scales) {
return Object.fromEntries(
Object.entries(channels).map(([name, {scale: scaleName, value}]) => {
let scale;
if (scaleName !== undefined) {
scale = scales[scaleName];
}
return [name, scale === undefined ? value : map(value, scale)];
const scale = scaleName == null ? null : scales[scaleName];
return [name, scale == null ? value : map(value, scale)];
})
);
}

// If the channel uses the "auto" scale (or equivalently true), infer the scale
// from the channel name and the provided values. For color and symbol channels,
// no scale is applied if the values are literal; however for symbols, we must
// promote symbol names (e.g., "plus") to symbol implementations (symbolPlus).
// Note: mutates channel!
export function inferChannelScale(name, channel) {
const {scale, value} = channel;
if (scale === true || scale === "auto") {
switch (name) {
case "fill":
case "stroke":
case "color":
channel.scale = isEvery(value, isColor) ? null : "color";
break;
case "fillOpacity":
case "strokeOpacity":
channel.scale = "opacity";
break;
case "symbol":
if (isEvery(value, isSymbol)) {
channel.scale = null;
channel.value = map(value, maybeSymbol);
} else {
channel.scale = "symbol";
}
break;
default:
channel.scale = registry.has(name) ? name : null;
break;
}
} else if (scale != null && !registry.has(scale)) {
throw new Error(`unknown scale: ${scale}`);
}
return channel;
}

// Note: mutates channel.domain! This is set to a function so that it is lazily
// computed; i.e., if the scale’s domain is set explicitly, that takes priority
// over the sort option, and we don’t need to do additional work.
Expand Down
21 changes: 15 additions & 6 deletions src/mark.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {Channels, channelDomain, valueObject} from "./channel.js";
import {defined} from "./defined.js";
import {maybeFacetAnchor} from "./facet.js";
import {arrayify, isDomainSort, range} from "./options.js";
import {arrayify, isDomainSort, isOptions, range} from "./options.js";
import {keyword, maybeNamed} from "./options.js";
import {maybeProject} from "./projection.js";
import {maybeClip, styles} from "./style.js";
Expand Down Expand Up @@ -41,11 +41,20 @@ export class Mark {
if (extraChannels !== undefined) channels = {...maybeNamed(extraChannels), ...channels};
if (defaults !== undefined) channels = {...styles(this, options, defaults), ...channels};
this.channels = Object.fromEntries(
Object.entries(channels).filter(([name, {value, optional}]) => {
if (value != null) return true;
if (optional) return false;
throw new Error(`missing channel value: ${name}`);
})
Object.entries(channels)
.map(([name, channel]) => {
const {value} = channel;
if (isOptions(value)) {
channel = {...channel, value: value.value};
if (value.scale !== undefined) channel.scale = value.scale;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we guard against bad scale values?

For example, we want to throw an error if the user passes
stroke: {value: "field", scale: "x"}
or
strokeOpacity: {value: Math.random, scale: "x"}

I was thinking we should test that channel.scale === "auto", and that value.scale is one of ["color", null] if name is stroke or color, and one of ["symbol", null] if name is symbol. And error in all other cases?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You’re saying we should restrict which scales we allow to be bound to which channels? Why? Is that likely? What if someone invents a creative use for doing something we didn’t anticipate?

I do think it’d be reasonable to enforce that the scale, if non-nullish, is a valid scale name (and not, say scale: "foo"). But I’m not sure we should do any validation beyond that.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I don't think the symbol or color channel can be assigned to x, even in a creative way—since their ranges are not in the same space? x, y, r and length are numbers, so there might be cases where you'd want to mix those, maybe—but most probably it's going to be a mistake, or a misunderstanding. (I also tried true, false… these will be fixed if we make sure that scale is nullish or a key of scales.)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’ve added validation of the scale name when the channel is registered, but only that the scale name is known (in the scale registry). Note it was already the case that a mark could declare a channel bound to a scale that doesn’t exist; it was being treated as if the channel were not bound to a scale. Though since this PR offers users new control over how channels are bound to scales without implementing a custom mark, it seems reasonable to add some validation.

I understand that it doesn’t make sense to put scale: "x" on the fill channel, or some such. But I also don’t see that we need to protect against it. There are so many other ways you can break plot. Adding validation requires us to codify which scales are allowed for which channels; we haven’t yet formalized this policy, and I don’t see a strong reason to do it now. Let’s address this in the future if users actually trip on it.

}
return [name, channel];
})
.filter(([name, {value, optional}]) => {
if (value != null) return true;
if (optional) return false;
throw new Error(`missing channel value: ${name}`);
})
);
this.dx = +dx;
this.dy = +dy;
Expand Down
2 changes: 1 addition & 1 deletion src/marks/dot.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export class Dot extends Mark {
y: {value: y, scale: "y", optional: true},
r: {value: vr, scale: "r", filter: positive, optional: true},
rotate: {value: vrotate, optional: true},
symbol: {value: vsymbol, scale: "symbol", optional: true}
symbol: {value: vsymbol, scale: "auto", optional: true}
},
withDefaultSort(options),
defaults
Expand Down
19 changes: 7 additions & 12 deletions src/options.js
Original file line number Diff line number Diff line change
Expand Up @@ -336,23 +336,18 @@ export function isNumeric(values) {
}
}

export function isFirst(values, is) {
for (const value of values) {
if (value == null) continue;
return is(value);
}
}

// Whereas isFirst only tests the first defined value and returns undefined for
// an empty array, this tests all defined values and only returns true if all of
// them are valid colors. It also returns true for an empty array, and thus
// should generally be used in conjunction with isFirst.
// Returns true if every non-null value in the specified iterable of values
// passes the specified predicate, and there is at least one non-null value;
// returns false if at least one non-null value does not pass the specified
// predicate; otherwise returns undefined (as if all values are null).
export function isEvery(values, is) {
let every;
for (const value of values) {
if (value == null) continue;
if (!is(value)) return false;
every = true;
}
return true;
return every;
}

// Mostly relies on d3-color, with a few extra color keywords. Currently this
Expand Down
26 changes: 4 additions & 22 deletions src/plot.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {select} from "d3";
import {Channel} from "./channel.js";
import {Channel, inferChannelScale} from "./channel.js";
import {Context, create} from "./context.js";
import {Dimensions} from "./dimensions.js";
import {Facets, facetExclude, facetGroups, facetOrder, facetTranslate, facetFilter} from "./facet.js";
Expand Down Expand Up @@ -156,7 +156,7 @@ export function plot(options = {}) {
state.facets = update.facets;
}
if (update.channels !== undefined) {
inferChannelScale(update.channels, mark);
inferChannelScales(update.channels);
Object.assign(state.channels, update.channels);
for (const channel of Object.values(update.channels)) {
const {scale} = channel;
Expand Down Expand Up @@ -370,27 +370,9 @@ function applyScaleTransform(channel, options) {
// An initializer may generate channels without knowing how the downstream mark
// will use them. Marks are typically responsible associated scales with
// channels, but here we assume common behavior across marks.
function inferChannelScale(channels) {
function inferChannelScales(channels) {
for (const name in channels) {
const channel = channels[name];
let {scale} = channel;
if (scale === true) {
switch (name) {
case "fill":
case "stroke":
scale = "color";
break;
case "fillOpacity":
case "strokeOpacity":
case "opacity":
scale = "opacity";
break;
default:
scale = scaleRegistry.has(name) ? name : null;
break;
}
channel.scale = scale;
}
inferChannelScale(name, channels[name]);
}
}

Expand Down
26 changes: 2 additions & 24 deletions src/scales.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
import {parse as isoParse} from "isoformat";
import {
isColor,
isEvery,
isOrdinal,
isFirst,
isTemporal,
isTemporalString,
isNumericString,
Expand Down Expand Up @@ -34,7 +31,7 @@ import {
import {isDivergingScheme} from "./scales/schemes.js";
import {ScaleTime, ScaleUtc} from "./scales/temporal.js";
import {ScaleOrdinal, ScalePoint, ScaleBand, ordinalImplicit} from "./scales/ordinal.js";
import {isSymbol, maybeSymbol} from "./symbols.js";
import {maybeSymbol} from "./symbols.js";
import {warn} from "./warnings.js";

export function Scales(
Expand Down Expand Up @@ -406,20 +403,8 @@ function inferScaleType(key, channels, {type, domain, range, scheme, pivot, proj
// If there’s no data (and no type) associated with this scale, don’t create a scale.
if (domain === undefined && !channels.some(({value}) => value !== undefined)) return;

const kind = registry.get(key);

// For color scales, if no range or scheme is specified and all associated
// defined values (from the domain if present, and otherwise from channels)
// are valid colors, then default to the identity scale. This allows, for
// example, a fill channel to return literal colors; without this, the colors
// would be remapped to a categorical scheme!
if (kind === color && range === undefined && scheme === undefined && isAll(domain, channels, isColor))
return "identity";

// Similarly for symbols…
if (kind === symbol && range === undefined && isAll(domain, channels, isSymbol)) return "identity";

// Some scales have default types.
const kind = registry.get(key);
if (kind === radius) return "sqrt";
if (kind === opacity || kind === length) return "linear";
if (kind === symbol) return "ordinal";
Expand Down Expand Up @@ -461,13 +446,6 @@ function asOrdinalType(kind) {
}
}

function isAll(domain, channels, is) {
return domain !== undefined
? isFirst(domain, is) && isEvery(domain, is)
: channels.some(({value}) => value !== undefined && isFirst(value, is)) &&
channels.every(({value}) => value === undefined || isEvery(value, is));
}

export function isTemporalScale({type}) {
return type === "time" || type === "utc";
}
Expand Down
4 changes: 2 additions & 2 deletions src/style.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,9 @@ export function styles(
title: {value: title, optional: true},
href: {value: href, optional: true},
ariaLabel: {value: variaLabel, optional: true},
fill: {value: vfill, scale: "color", optional: true},
fill: {value: vfill, scale: "auto", optional: true},
fillOpacity: {value: vfillOpacity, scale: "opacity", optional: true},
stroke: {value: vstroke, scale: "color", optional: true},
stroke: {value: vstroke, scale: "auto", optional: true},
strokeOpacity: {value: vstrokeOpacity, scale: "opacity", optional: true},
strokeWidth: {value: vstrokeWidth, optional: true},
opacity: {value: vopacity, scale: "opacity", optional: true}
Expand Down
4 changes: 2 additions & 2 deletions test/marks/area-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ it("area(data, {fill}) allows fill to be a variable color", () => {
assert.strictEqual(area.fill, undefined);
const {fill} = area.channels;
assert.strictEqual(fill.value, "x");
assert.strictEqual(fill.scale, "color");
assert.strictEqual(fill.scale, "auto");
});

it("area(data, {fill}) implies a default z channel if fill is variable", () => {
Expand All @@ -98,7 +98,7 @@ it("area(data, {stroke}) allows stroke to be a variable color", () => {
assert.strictEqual(area.stroke, undefined);
const {stroke} = area.channels;
assert.strictEqual(stroke.value, "x");
assert.strictEqual(stroke.scale, "color");
assert.strictEqual(stroke.scale, "auto");
});

it("area(data, {stroke}) implies a default z channel if stroke is variable", () => {
Expand Down
8 changes: 4 additions & 4 deletions test/marks/bar-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ it("barX(data, {fill}) allows fill to be a variable color", () => {
assert.strictEqual(bar.fill, undefined);
const {fill} = bar.channels;
assert.strictEqual(fill.value, "x");
assert.strictEqual(fill.scale, "color");
assert.strictEqual(fill.scale, "auto");
});

it("barX(data, {stroke}) allows stroke to be a constant color", () => {
Expand All @@ -80,7 +80,7 @@ it("barX(data, {stroke}) allows stroke to be a variable color", () => {
assert.strictEqual(bar.stroke, undefined);
const {stroke} = bar.channels;
assert.strictEqual(stroke.value, "x");
assert.strictEqual(stroke.scale, "color");
assert.strictEqual(stroke.scale, "auto");
});

it("barX(data, {x, y}) defaults x1 to zero and x2 to x", () => {
Expand Down Expand Up @@ -162,7 +162,7 @@ it("barY(data, {fill}) allows fill to be a variable color", () => {
assert.strictEqual(bar.fill, undefined);
const {fill} = bar.channels;
assert.strictEqual(fill.value, "x");
assert.strictEqual(fill.scale, "color");
assert.strictEqual(fill.scale, "auto");
});

it("barY(data, {stroke}) allows stroke to be a constant color", () => {
Expand All @@ -180,7 +180,7 @@ it("barY(data, {stroke}) allows stroke to be a variable color", () => {
assert.strictEqual(bar.stroke, undefined);
const {stroke} = bar.channels;
assert.strictEqual(stroke.value, "x");
assert.strictEqual(stroke.scale, "color");
assert.strictEqual(stroke.scale, "auto");
});

it("barY(data, {x, y}) defaults y1 to zero and y2 to y", () => {
Expand Down
8 changes: 4 additions & 4 deletions test/marks/cell-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ it("cell(data, {fill}) allows fill to be a variable color", () => {
assert.strictEqual(cell.fill, undefined);
const {fill} = cell.channels;
assert.strictEqual(fill.value, "x");
assert.strictEqual(fill.scale, "color");
assert.strictEqual(fill.scale, "auto");
});

it("cell(data, {stroke}) allows stroke to be a constant color", () => {
Expand All @@ -85,7 +85,7 @@ it("cell(data, {stroke}) allows stroke to be a variable color", () => {
assert.strictEqual(cell.stroke, undefined);
const {stroke} = cell.channels;
assert.strictEqual(stroke.value, "x");
assert.strictEqual(stroke.scale, "color");
assert.strictEqual(stroke.scale, "auto");
});

it("cellX() defaults x to identity and y to null", () => {
Expand All @@ -102,7 +102,7 @@ it("cellX() defaults x to identity and y to null", () => {
);
assert.deepStrictEqual(
Object.values(cell.channels).map((c) => c.scale),
["color", "x"]
["auto", "x"]
);
assert.strictEqual(cell.channels.x.type, "band");
});
Expand All @@ -121,7 +121,7 @@ it("cellY() defaults y to identity and x to null", () => {
);
assert.deepStrictEqual(
Object.values(cell.channels).map((c) => c.scale),
["color", "y"]
["auto", "y"]
);
assert.strictEqual(cell.channels.y.type, "band");
});
Expand Down
4 changes: 2 additions & 2 deletions test/marks/dot-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ it("dot(data, {fill}) allows fill to be a variable color", () => {
assert.strictEqual(dot.fill, undefined);
const {fill} = dot.channels;
assert.strictEqual(fill.value, "x");
assert.strictEqual(fill.scale, "color");
assert.strictEqual(fill.scale, "auto");
});

it("dot(data, {fill}) defaults stroke to undefined if fill is not none", () => {
Expand All @@ -103,7 +103,7 @@ it("dot(data, {stroke}) allows stroke to be a variable color", () => {
assert.strictEqual(dot.stroke, undefined);
const {stroke} = dot.channels;
assert.strictEqual(stroke.value, "x");
assert.strictEqual(stroke.scale, "color");
assert.strictEqual(stroke.scale, "auto");
});

it("dot(data, {stroke}) defaults strokeWidth to 1.5 if stroke is defined", () => {
Expand Down
4 changes: 2 additions & 2 deletions test/marks/line-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ it("line(data, {fill}) allows fill to be a variable color", () => {
assert.strictEqual(line.fill, undefined);
const {fill} = line.channels;
assert.strictEqual(fill.value, "x");
assert.strictEqual(fill.scale, "color");
assert.strictEqual(fill.scale, "auto");
});

it("line(data, {fill}) implies a default z channel if fill is variable", () => {
Expand Down Expand Up @@ -100,7 +100,7 @@ it("line(data, {stroke}) allows stroke to be a variable color", () => {
assert.strictEqual(line.stroke, undefined);
const {stroke} = line.channels;
assert.strictEqual(stroke.value, "x");
assert.strictEqual(stroke.scale, "color");
assert.strictEqual(stroke.scale, "auto");
});

it("line(data, {stroke}) implies a default z channel if stroke is variable", () => {
Expand Down
Loading