Skip to content

Commit

Permalink
fix: use expression to generalize zeroOrMinOrMax only when there is a…
Browse files Browse the repository at this point in the history
… scale
  • Loading branch information
kanitw committed May 13, 2024
1 parent 24f9979 commit 94340a0
Show file tree
Hide file tree
Showing 5 changed files with 93 additions and 49 deletions.
9 changes: 6 additions & 3 deletions src/compile/mark/encode/invalid.ts
Expand Up @@ -8,6 +8,7 @@ import {getScaleInvalidDataMode} from '../../invalid/ScaleInvalidDataMode';
import {ScaleComponent} from '../../scale/component';
import {zeroOrMinOrMax} from './zeroOrMinOrMax';
import {MidPointParams} from './valueref';
import {Config} from '../../../config';

export function getConditionalValueRefForIncludingInvalidValue<
C extends PositionScaleChannel | PolarPositionScaleChannel | NonPositionScaleChannel
Expand Down Expand Up @@ -38,7 +39,7 @@ export function getConditionalValueRefForIncludingInvalidValue<
const includeAs: ScaleInvalidDataIncludeAs<C> = config.scale.invalid?.[scaleChannel] ?? 'zero-or-min';
return {
test: fieldValidPredicate(vgField(fieldDef, {expr: 'datum'}), false),
...refForInvalidValues(scaleChannel, includeAs, scale, scaleName)
...refForInvalidValues(scaleChannel, includeAs, scale, scaleName, config)
};
}
return undefined;
Expand All @@ -48,7 +49,8 @@ function refForInvalidValues<C extends PositionScaleChannel | PolarPositionScale
channel: C,
includeAs: ScaleInvalidDataIncludeAs<C>,
scale: ScaleComponent,
scaleName: string
scaleName: string,
config: Config
): VgValueRef {
if (isScaleInvalidDataIncludeAsValue(includeAs)) {
const {value} = includeAs;
Expand All @@ -59,6 +61,7 @@ function refForInvalidValues<C extends PositionScaleChannel | PolarPositionScale
mainChannel: channel,
scale,
scaleName,
mode: includeAs === 'zero-or-min' ? 'zeroOrMin' : 'min'
mode: includeAs === 'zero-or-min' ? 'zeroOrMin' : 'min',
config
});
}
5 changes: 3 additions & 2 deletions src/compile/mark/encode/position-point.ts
Expand Up @@ -141,13 +141,14 @@ export function pointPositionDefaultRef({

switch (defaultPos) {
case 'zeroOrMin':
return zeroOrMinOrMax({scaleName, scale, mode: 'zeroOrMin', mainChannel});
return zeroOrMinOrMax({scaleName, scale, mode: 'zeroOrMin', mainChannel, config});
case 'zeroOrMax':
return zeroOrMinOrMax({
scaleName,
scale,
mode: {zeroOrMax: {widthSignal: model.width.signal, heightSignal: model.height.signal}},
mainChannel
mainChannel,
config
});
case 'mid': {
const sizeRef = model[getSizeChannel(channel)];
Expand Down
92 changes: 59 additions & 33 deletions src/compile/mark/encode/zeroOrMinOrMax.ts
@@ -1,54 +1,80 @@
import {NonPositionScaleChannel, PolarPositionScaleChannel, PositionScaleChannel} from '../../../channel';
import {Config} from '../../../config';
import {VgValueRef} from '../../../vega.schema';
import {ScaleComponent} from '../../scale/component';

export function zeroOrMinOrMax({
scaleName,
scale,
mode,
mainChannel
mainChannel,
config
}: {
scaleName: string;
scale: ScaleComponent;
mode: 'min' | 'zeroOrMin' | {zeroOrMax: {widthSignal: string; heightSignal: string}};
mainChannel: PositionScaleChannel | PolarPositionScaleChannel | NonPositionScaleChannel;
config: Config;
}): VgValueRef {
if (mode !== 'min' && scaleName && scale.domainDefinitelyIncludesZero()) {
return {
scale: scaleName,
value: 0
};
}
const domain = `domain('${scaleName}')`;
const min = `${domain}[0]`;
const max = `peek(${domain})`; // peek = the last item of the array

if (mode === 'zeroOrMin' || mode === 'min') {
switch (mainChannel) {
case 'radius':
case 'theta':
case 'x':
return {value: 0};
case 'y':
return {field: {group: 'height'}};
default:
return {signal: `scale('${scaleName}', domain('${scaleName}')[0])`}; // encode the scale domain min
}
} else {
// zeroOrMax
switch (mainChannel) {
case 'radius': {
const {widthSignal, heightSignal} = mode.zeroOrMax;
// max of radius is min(width, height) / 2
if (scale && scaleName) {
// If there is a scale (and hence its name)
const domainHasZero = scale.domainHasZero();
if (mode === 'min') {
return {signal: `scale('${scaleName}', ${min})`}; // encode the scale domain min
} else {
// zeroOrMin or zeroOrMax mode
if (domainHasZero === 'definitely') {
return {
signal: `min(${widthSignal},${heightSignal})/2`
scale: scaleName,
value: 0
};
} else if (domainHasZero === 'maybe') {
if (mode === 'zeroOrMin') {
return {signal: `scale('${scaleName}', inrange(0, ${domain}) ? 0 : ${min})`}; // encode the scale domain min
} else {
return {signal: `scale('${scaleName}', inrange(0, ${domain}) ? 0 : ${max})`}; // encode the scale domain max
}
}
}
}

const isMin = mode === 'zeroOrMin' || mode === 'min';
switch (mainChannel) {
case 'radius': {
if (mode === 'min' || mode === 'zeroOrMin') {
return {value: 0}; // min value
}
case 'theta':
return {signal: '2*PI'};
case 'x':
return {field: {group: 'width'}};
case 'y':
return {value: 0};
default:
return {signal: `scale('${scaleName}', domain('${scaleName}')[1])`}; // encode the scale domain max
const {widthSignal, heightSignal} = mode.zeroOrMax;
// max of radius is min(width, height) / 2
return {
signal: `min(${widthSignal},${heightSignal})/2`
};
}
case 'theta':
case 'angle':
return isMin ? {value: 0} : {signal: '2*PI'};
case 'x':
return isMin ? {value: 0} : {field: {group: 'width'}};
case 'y':
return isMin ? {field: {group: 'height'}} : {value: 0};
case 'color':
case 'fill':
case 'stroke':
return {value: '#aaa'};
case 'opacity':
case 'fillOpacity':
case 'strokeOpacity':
return {value: config.scale[isMin ? 'minOpacity' : 'maxOpacity']};
case 'strokeWidth':
return {value: config.scale[isMin ? 'minStrokeWidth' : 'maxStrokeWidth']};
case 'size':
return {value: config.scale[isMin ? 'minSize' : 'maxSize']};
case 'strokeDash':
case 'shape':
return {value: null};
}
}
34 changes: 24 additions & 10 deletions src/compile/scale/component.ts
Expand Up @@ -31,23 +31,37 @@ export class ScaleComponent extends Split<ScaleComponentProps> {
}

/**
* Whether the scale definitely includes zero in the domain
* Whether the scale definitely includes or not include zero in the domain
*/
public domainDefinitelyIncludesZero() {
if (contains([ScaleType.LOG, ScaleType.TIME, ScaleType.UTC], this.get('type'))) {
public domainHasZero(): 'definitely' | 'definitely-not' | 'maybe' {
const scaleType = this.get('type');
if (contains([ScaleType.LOG, ScaleType.TIME, ScaleType.UTC], scaleType)) {
// Log scales cannot have zero.
// Zero in time scale is arbitrary, and does not affect ratio.
// (Time is an interval level of measurement, not ratio).
// See https://en.wikipedia.org/wiki/Level_of_measurement for more info.
return false;
return 'definitely-not';
}
if (this.get('zero') !== false) {
return true;

const scaleZero = this.get('zero');
if (
scaleZero === true ||
// If zero is undefined, linear/sqrt/pow scales have zero by default.
(scaleType === undefined && contains([ScaleType.LINEAR, ScaleType.SQRT, ScaleType.POW], scaleType))
) {
return 'definitely';
}
return some(
this.get('domains'),
d => isArray(d) && d.length === 2 && isNumber(d[0]) && d[0] <= 0 && isNumber(d[1]) && d[1] >= 0
);

const domains = this.get('domains');

if (domains.length > 0) {
const hasDomainWithZero = some(
domains,
d => isArray(d) && d.length === 2 && isNumber(d[0]) && d[0] <= 0 && isNumber(d[1]) && d[1] >= 0
);
return hasDomainWithZero ? 'definitely' : 'definitely-not';
}
return 'maybe';
}
}

Expand Down
2 changes: 1 addition & 1 deletion test/compile/mark/point.test.ts
Expand Up @@ -238,7 +238,7 @@ describe('Mark: Point', () => {

it('should test for invalid values on y', () => {
expect(props.y).toEqual([
{field: {group: 'height'}, test: '!isValid(datum["yield"]) || !isFinite(+datum["yield"])'},
{scale: 'y', value: 0, test: '!isValid(datum["yield"]) || !isFinite(+datum["yield"])'},
{scale: Y, field: 'yield'}
]);
});
Expand Down

0 comments on commit 94340a0

Please sign in to comment.