Skip to content

Commit

Permalink
fix: scale-bound selections should respect reversals (#7163)
Browse files Browse the repository at this point in the history 
* fix + test final argument to pan/zoom symlog/pow expr functions.

* fix: translating scale-bound intervals should respect reversals

* fix: reverse selection extent for descending-ordered scale domains
  • Loading branch information
@arvind
arvind committed Jan 7, 2021
1 parent 55d6238 commit bafca07
Show file tree
Hide file tree
Showing 9 changed files with 217 additions and 80 deletions.
4 changes: 2 additions & 2 deletions src/compile/data/bin.ts
View file
Expand Up @@ -8,7 +8,7 @@ import {BinTransform} from '../../transform';
import {Dict, duplicate, hash, isEmpty, keys, replacePathInField, unique, vals, varName} from '../../util';
import {binFormatExpression} from '../format';
import {isUnitModel, Model, ModelWithField} from '../model';
import {parseSelectionBinExtent} from '../selection/parse';
import {parseSelectionExtent} from '../selection/parse';
import {NonPositionScaleChannel, PositionChannel} from './../../channel';
import {DataFlowNode} from './dataflow';

Expand Down Expand Up @@ -69,7 +69,7 @@ function createBinComponent(t: TypedFieldDef<string> | BinTransform, bin: boolea
if (isSelectionExtent(normalizedBin.extent)) {
const ext = normalizedBin.extent;
const selName = ext.selection;
span = parseSelectionBinExtent(model.getSelectionComponent(varName(selName), selName), ext);
span = parseSelectionExtent(model.getSelectionComponent(varName(selName), selName), ext);
delete normalizedBin.extent; // Vega-Lite selection extent map to Vega's span property.
}

Expand Down
8 changes: 3 additions & 5 deletions src/compile/scale/assemble.ts
View file
Expand Up @@ -31,12 +31,10 @@ export function assembleScalesForModel(model: Model): VgScale[] {
const {name, type, selectionExtent, domains: _d, range: _r, reverse, ...otherScaleProps} = scale;
const range = assembleScaleRange(scale.range, name, channel, model);

let domainRaw;
if (selectionExtent) {
domainRaw = assembleSelectionScaleDomain(model, selectionExtent);
}

const domain = assembleDomain(model, channel);
const domainRaw = selectionExtent
? assembleSelectionScaleDomain(model, selectionExtent, scaleComponent, domain)
: null;

scales.push({
name,
Expand Down
22 changes: 18 additions & 4 deletions src/compile/selection/assemble.ts
View file
Expand Up @@ -3,14 +3,16 @@ import {selector as parseSelector} from 'vega-event-selector';
import {identity, isArray, stringValue} from 'vega-util';
import {MODIFY, STORE, unitName, VL_SELECTION_RESOLVE, TUPLE, selectionCompilers} from '.';
import {dateTimeToExpr, isDateTime, dateTimeToTimestamp} from '../../datetime';
import {hasContinuousDomain} from '../../scale';
import {SelectionInit, SelectionInitInterval, SelectionExtent} from '../../selection';
import {keys, vals, varName} from '../../util';
import {VgData} from '../../vega.schema';
import {VgData, VgDomain} from '../../vega.schema';
import {FacetModel} from '../facet';
import {LayerModel} from '../layer';
import {isUnitModel, Model} from '../model';
import {ScaleComponent} from '../scale/component';
import {UnitModel} from '../unit';
import {parseSelectionBinExtent} from './parse';
import {parseSelectionExtent} from './parse';

export function assembleInit(
init: readonly (SelectionInit | readonly SelectionInit[] | SelectionInitInterval)[] | SelectionInit,
Expand Down Expand Up @@ -157,10 +159,22 @@ export function assembleLayerSelectionMarks(model: LayerModel, marks: any[]): an
return marks;
}

export function assembleSelectionScaleDomain(model: Model, extent: SelectionExtent): SignalRef {
export function assembleSelectionScaleDomain(
model: Model,
extent: SelectionExtent,
scaleCmpt: ScaleComponent,
domain: VgDomain
): SignalRef {
const name = extent.selection;
const selCmpt = model.getSelectionComponent(name, varName(name));
return {signal: parseSelectionBinExtent(selCmpt, extent)};
const parsedExtent = parseSelectionExtent(selCmpt, extent);

return {
signal:
hasContinuousDomain(scaleCmpt.get('type')) && isArray(domain) && domain[0] > domain[1]
? `isValid(${parsedExtent}) && reverse(${parsedExtent})`
: parsedExtent
};
}

function cleanupEmptyOnArray(signals: Signal[]) {
Expand Down
2 changes: 1 addition & 1 deletion src/compile/selection/parse.ts
View file
Expand Up @@ -94,7 +94,7 @@ export function parseSelectionPredicate(
);
}

export function parseSelectionBinExtent(selCmpt: SelectionComponent, extent: SelectionExtent) {
export function parseSelectionExtent(selCmpt: SelectionComponent, extent: SelectionExtent) {
const encoding = extent['encoding'];
let field = extent['field'];

Expand Down
15 changes: 10 additions & 5 deletions src/compile/selection/translate.ts
View file
Expand Up @@ -84,7 +84,8 @@ function onDelta(
const sizeSg = model.getSizeSignalRef(size).signal;
const scaleCmpt = model.getScaleComponent(channel);
const scaleType = scaleCmpt.get('type');
const sign = hasScales && channel === X ? '-' : ''; // Invert delta when panning x-scales.
const reversed = scaleCmpt.get('reverse'); // scale parsing sets this flag for fieldDef.sort
const sign = !hasScales ? '' : channel === X ? (reversed ? '' : '-') : reversed ? '-' : '';
const extent = `${anchor}.extent_${channel}`;
const offset = `${sign}${delta}.${channel} / ` + (hasScales ? `${sizeSg}` : `span(${extent})`);
const panFn = !hasScales
Expand All @@ -96,10 +97,14 @@ function onDelta(
: scaleType === 'pow'
? 'panPow'
: 'panLinear';
const update =
`${panFn}(${extent}, ${offset}` +
(hasScales && scaleType === 'pow' ? `, ${scaleCmpt.get('exponent') ?? 1}` : '') +
')';
const arg = !hasScales
? ''
: scaleType === 'pow'
? `, ${scaleCmpt.get('exponent') ?? 1}`
: scaleType === 'symlog'
? `, ${scaleCmpt.get('constant') ?? 1}`
: '';
const update = `${panFn}(${extent}, ${offset}${arg})`;

signal.on.push({
events: {signal: delta},
Expand Down
12 changes: 8 additions & 4 deletions src/compile/selection/zoom.ts
View file
Expand Up @@ -98,10 +98,14 @@ function onDelta(
: scaleType === 'pow'
? 'zoomPow'
: 'zoomLinear';
const update =
`${zoomFn}(${base}, ${anchor}, ${delta}` +
(hasScales && scaleType === 'pow' ? `, ${scaleCmpt.get('exponent') ?? 1}` : '') +
')';
const arg = !hasScales
? ''
: scaleType === 'pow'
? `, ${scaleCmpt.get('exponent') ?? 1}`
: scaleType === 'symlog'
? `, ${scaleCmpt.get('constant') ?? 1}`
: '';
const update = `${zoomFn}(${base}, ${anchor}, ${delta}${arg})`;

signal.on.push({
events: {signal: delta},
Expand Down
36 changes: 29 additions & 7 deletions test/compile/selection/scales.test.ts
View file
Expand Up @@ -6,7 +6,7 @@ import {assembleTopLevelSignals, assembleUnitSelectionSignals} from '../../../sr
import {UnitModel} from '../../../src/compile/unit';
import * as log from '../../../src/log';
import {Domain} from '../../../src/scale';
import {parseConcatModel, parseModel, parseUnitModelWithScale} from '../../util';
import {parseConcatModel, parseModel, parseUnitModelWithScaleAndSelection} from '../../util';

describe('Selection + Scales', () => {
describe('selectionExtent', () => {
Expand Down Expand Up @@ -128,7 +128,7 @@ describe('Selection + Scales', () => {
});

it('should handle nested field references', () => {
let model: Model = parseUnitModelWithScale({
let model: Model = parseUnitModelWithScaleAndSelection({
params: [
{
name: 'grid',
Expand All @@ -151,7 +151,6 @@ describe('Selection + Scales', () => {
}
}
});
model.parseSelections();

let scales = assembleScalesForModel(model);
expect(scales[0]).toHaveProperty('domainRaw');
Expand Down Expand Up @@ -222,6 +221,31 @@ describe('Selection + Scales', () => {
expect(scales[0]).toHaveProperty('domainRaw');
expect(scales[0].domainRaw).toEqual({signal: 'brush["nested\\\\.a"]'});
});

it('should respect ordering of explicit scale domains', () => {
const model = parseUnitModelWithScaleAndSelection({
mark: 'point',
encoding: {
x: {
type: 'quantitative',
field: 'Horsepower',
scale: {domain: [250, 0]}
},
y: {type: 'quantitative', field: 'Miles_per_Gallon'}
},
params: [
{
name: 'pan',
select: 'interval',
bind: 'scales'
}
]
});

const scales = assembleScalesForModel(model);
expect(scales[0]).toHaveProperty('domainRaw');
expect(scales[0].domainRaw).toEqual({signal: 'isValid(pan["Horsepower"]) && reverse(pan["Horsepower"])'});
});
});

describe('signals', () => {
Expand Down Expand Up @@ -364,7 +388,7 @@ describe('Selection + Scales', () => {
it(
'should not bind for unavailable/unsupported scales',
log.wrap(localLogger => {
let model = parseUnitModelWithScale({
parseUnitModelWithScaleAndSelection({
data: {url: 'data/cars.json'},
params: [
{
Expand All @@ -378,10 +402,9 @@ describe('Selection + Scales', () => {
y: {field: 'Miles_per_Gallon', type: 'quantitative'}
}
});
model.parseSelections();
expect(localLogger.warns[0]).toEqual(log.message.cannotProjectOnChannelWithoutField(X));

model = parseUnitModelWithScale({
parseUnitModelWithScaleAndSelection({
data: {url: 'data/cars.json'},
params: [
{
Expand All @@ -396,7 +419,6 @@ describe('Selection + Scales', () => {
y: {field: 'Miles_per_Gallon', type: 'quantitative'}
}
});
model.parseSelections();
expect(localLogger.warns[1]).toEqual(log.message.SCALE_BINDINGS_CONTINUOUS);
})
);
Expand Down
117 changes: 91 additions & 26 deletions test/compile/selection/translate.test.ts
View file
Expand Up @@ -2,15 +2,21 @@ import {selector as parseSelector} from 'vega-event-selector';
import {assembleUnitSelectionSignals} from '../../../src/compile/selection/assemble';
import {parseUnitSelection} from '../../../src/compile/selection/parse';
import translate from '../../../src/compile/selection/translate';
import {ScaleType} from '../../../src/scale';
import {Scale} from '../../../src/scale';
import {Sort} from '../../../src/sort';
import {parseUnitModel} from '../../util';

function getModel(xscale?: ScaleType, yscale?: ScaleType) {
function getModel(
xscale: Scale = {type: 'linear'},
yscale: Scale = {type: 'linear'},
xsort?: Sort<string>,
ysort?: Sort<string>
) {
const model = parseUnitModel({
mark: 'circle',
encoding: {
x: {field: 'Horsepower', type: 'quantitative', scale: {type: xscale ?? 'linear'}},
y: {field: 'Miles_per_Gallon', type: 'quantitative', scale: {type: yscale ?? 'linear'}},
x: {field: 'Horsepower', type: 'quantitative', scale: xscale, ...(xsort ? {sort: xsort} : {})},
y: {field: 'Miles_per_Gallon', type: 'quantitative', scale: yscale, ...(ysort ? {sort: ysort} : {})},
color: {field: 'Origin', type: 'nominal'}
}
});
Expand Down Expand Up @@ -184,7 +190,7 @@ describe('Translate Selection Transform', () => {
'clampRange(panLinear(four_translate_anchor.extent_y, four_translate_delta.y / span(four_translate_anchor.extent_y)), 0, height)'
});

const model2 = getModel('log', 'pow').model;
const model2 = getModel({type: 'log'}, {type: 'pow'}).model;
model2.component.selection = {four: selCmpts['four']};
signals = assembleUnitSelectionSignals(model2, []);
expect(signals.filter(s => s.name === 'four_x')[0].on).toContainEqual({
Expand All @@ -200,35 +206,94 @@ describe('Translate Selection Transform', () => {
});
});

it('builds panLinear exprs for scale-bound intervals', () => {
const {model, selCmpts} = getModel();
model.component.selection = {six: selCmpts['six']};
const signals = assembleUnitSelectionSignals(model, []);
describe('scale-bound intervals', () => {
it('builds panLinear exprs', () => {
const {model, selCmpts} = getModel();
model.component.selection = {six: selCmpts['six']};
const signals = assembleUnitSelectionSignals(model, []);

expect(signals.filter(s => s.name === 'six_Horsepower')[0].on).toContainEqual({
events: {signal: 'six_translate_delta'},
update: 'panLinear(six_translate_anchor.extent_x, -six_translate_delta.x / width)'
expect(signals.filter(s => s.name === 'six_Horsepower')[0].on).toContainEqual({
events: {signal: 'six_translate_delta'},
update: 'panLinear(six_translate_anchor.extent_x, -six_translate_delta.x / width)'
});

expect(signals.filter(s => s.name === 'six_Miles_per_Gallon')[0].on).toContainEqual({
events: {signal: 'six_translate_delta'},
update: 'panLinear(six_translate_anchor.extent_y, six_translate_delta.y / height)'
});
});

expect(signals.filter(s => s.name === 'six_Miles_per_Gallon')[0].on).toContainEqual({
events: {signal: 'six_translate_delta'},
update: 'panLinear(six_translate_anchor.extent_y, six_translate_delta.y / height)'
it('builds panLog exprs', () => {
const {model, selCmpts} = getModel({type: 'log'});
model.component.selection = {six: selCmpts['six']};
const signals = assembleUnitSelectionSignals(model, []);

expect(signals.filter(s => s.name === 'six_Horsepower')[0].on).toContainEqual({
events: {signal: 'six_translate_delta'},
update: 'panLog(six_translate_anchor.extent_x, -six_translate_delta.x / width)'
});
});
});

it('builds panLog/panPow exprs for scale-bound intervals', () => {
const {model, selCmpts} = getModel('log', 'pow');
model.component.selection = {six: selCmpts['six']};
const signals = assembleUnitSelectionSignals(model, []);
it('builds panSymlog exprs', () => {
const {model, selCmpts} = getModel({type: 'symlog'}, {type: 'symlog', constant: 0.5});
model.component.selection = {six: selCmpts['six']};
const signals = assembleUnitSelectionSignals(model, []);

expect(signals.filter(s => s.name === 'six_Horsepower')[0].on).toContainEqual({
events: {signal: 'six_translate_delta'},
update: 'panSymlog(six_translate_anchor.extent_x, -six_translate_delta.x / width, 1)'
});

expect(signals.filter(s => s.name === 'six_Horsepower')[0].on).toContainEqual({
events: {signal: 'six_translate_delta'},
update: 'panLog(six_translate_anchor.extent_x, -six_translate_delta.x / width)'
expect(signals.filter(s => s.name === 'six_Miles_per_Gallon')[0].on).toContainEqual({
events: {signal: 'six_translate_delta'},
update: 'panSymlog(six_translate_anchor.extent_y, six_translate_delta.y / height, 0.5)'
});
});

expect(signals.filter(s => s.name === 'six_Miles_per_Gallon')[0].on).toContainEqual({
events: {signal: 'six_translate_delta'},
update: 'panPow(six_translate_anchor.extent_y, six_translate_delta.y / height, 1)'
it('builds panPow exprs', () => {
const {model, selCmpts} = getModel({type: 'pow'}, {type: 'pow', exponent: 2});
model.component.selection = {six: selCmpts['six']};
const signals = assembleUnitSelectionSignals(model, []);

expect(signals.filter(s => s.name === 'six_Horsepower')[0].on).toContainEqual({
events: {signal: 'six_translate_delta'},
update: 'panPow(six_translate_anchor.extent_x, -six_translate_delta.x / width, 1)'
});

expect(signals.filter(s => s.name === 'six_Miles_per_Gallon')[0].on).toContainEqual({
events: {signal: 'six_translate_delta'},
update: 'panPow(six_translate_anchor.extent_y, six_translate_delta.y / height, 2)'
});
});

it('respects reversals', () => {
let {model, selCmpts} = getModel({type: 'linear', reverse: true}, {type: 'linear', reverse: true});
model.component.selection = {six: selCmpts['six']};
let signals = assembleUnitSelectionSignals(model, []);

expect(signals.filter(s => s.name === 'six_Horsepower')[0].on).toContainEqual({
events: {signal: 'six_translate_delta'},
update: 'panLinear(six_translate_anchor.extent_x, six_translate_delta.x / width)'
});

expect(signals.filter(s => s.name === 'six_Miles_per_Gallon')[0].on).toContainEqual({
events: {signal: 'six_translate_delta'},
update: 'panLinear(six_translate_anchor.extent_y, -six_translate_delta.y / height)'
});

({model, selCmpts} = getModel({type: 'linear'}, {type: 'linear'}, 'descending', 'descending'));
model.component.selection = {six: selCmpts['six']};
signals = assembleUnitSelectionSignals(model, []);

expect(signals.filter(s => s.name === 'six_Horsepower')[0].on).toContainEqual({
events: {signal: 'six_translate_delta'},
update: 'panLinear(six_translate_anchor.extent_x, six_translate_delta.x / width)'
});

expect(signals.filter(s => s.name === 'six_Miles_per_Gallon')[0].on).toContainEqual({
events: {signal: 'six_translate_delta'},
update: 'panLinear(six_translate_anchor.extent_y, -six_translate_delta.y / height)'
});
});
});
});
Expand Down

0 comments on commit bafca07

Please sign in to comment.