Skip to content

Commit

Permalink
fix: correct references to flattened fields in selection signals. (#5351
Browse files Browse the repository at this point in the history 
)

* Update formatparse to flatten non-encoding selection projections.

* Correct references to flattened fields in selection signals.

* Escape flattened field names in tuple_fields signal.

* Extract how fields should be parsed implicitly.
  • Loading branch information
@arvind
arvind authored Sep 10, 2019
1 parent 849e279 commit 88333ef
Show file tree
Hide file tree
Showing 14 changed files with 404 additions and 156 deletions.
236 changes: 124 additions & 112 deletions src/compile/data/formatparse.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,129 @@ function parseExpression(field: string, parse: string): string {
}
}

export function getImplicitFromFilterTransform(transform: FilterTransform) {
const implicit: Dict<string> = {};
forEachLeaf(transform.filter, filter => {
if (isFieldPredicate(filter)) {
// Automatically add a parse node for filters with filter objects
let val: string | number | boolean | DateTime = null;

// For EqualFilter, just use the equal property.
// For RangeFilter and OneOfFilter, all array members should have
// the same type, so we only use the first one.
if (isFieldEqualPredicate(filter)) {
val = filter.equal;
} else if (isFieldRangePredicate(filter)) {
val = filter.range[0];
} else if (isFieldOneOfPredicate(filter)) {
val = (filter.oneOf || filter['in'])[0];
} // else -- for filter expression, we can't infer anything
if (val) {
if (isDateTime(val)) {
implicit[filter.field] = 'date';
} else if (isNumber(val)) {
implicit[filter.field] = 'number';
} else if (isString(val)) {
implicit[filter.field] = 'string';
}
}

if (filter.timeUnit) {
implicit[filter.field] = 'date';
}
}
});

return implicit;
}

/**
* Creates a parse node for implicit parsing from a model and updates ancestorParse.
*/
export function getImplicitFromEncoding(model: Model) {
const implicit: Dict<string> = {};

function add(fieldDef: TypedFieldDef<string>) {
if (isTimeFormatFieldDef(fieldDef)) {
implicit[fieldDef.field] = 'date';
} else if (
fieldDef.type === 'quantitative' &&
isMinMaxOp(fieldDef.aggregate) // we need to parse numbers to support correct min and max
) {
implicit[fieldDef.field] = 'number';
} else if (accessPathDepth(fieldDef.field) > 1) {
// For non-date/non-number (strings and booleans), derive a flattened field for a referenced nested field.
// (Parsing numbers / dates already flattens numeric and temporal fields.)
if (!(fieldDef.field in implicit)) {
implicit[fieldDef.field] = 'flatten';
}
} else if (isScaleFieldDef(fieldDef) && isSortField(fieldDef.sort) && accessPathDepth(fieldDef.sort.field) > 1) {
// Flatten fields that we sort by but that are not otherwise flattened.
if (!(fieldDef.sort.field in implicit)) {
implicit[fieldDef.sort.field] = 'flatten';
}
}
}

if (isUnitModel(model) || isFacetModel(model)) {
// Parse encoded fields
model.forEachFieldDef((fieldDef, channel) => {
if (isTypedFieldDef(fieldDef)) {
add(fieldDef);
} else {
const mainChannel = getMainRangeChannel(channel);
const mainFieldDef = model.fieldDef(mainChannel as SingleDefChannel) as TypedFieldDef<string>;
add({
...fieldDef,
type: mainFieldDef.type
});
}
});
}

// Parse quantitative dimension fields of path marks as numbers so that we sort them correctly.
if (isUnitModel(model)) {
const {mark, markDef, encoding} = model;
if (
isPathMark(mark) &&
// No need to sort by dimension if we have a connected scatterplot (order channel is present)
!model.encoding.order
) {
const dimensionChannel = markDef.orient === 'horizontal' ? 'y' : 'x';
const dimensionChannelDef = encoding[dimensionChannel];
if (
isFieldDef(dimensionChannelDef) &&
dimensionChannelDef.type === 'quantitative' &&
!(dimensionChannelDef.field in implicit)
) {
implicit[dimensionChannelDef.field] = 'number';
}
}
}

return implicit;
}

/**
* Creates a parse node for implicit parsing from a model and updates ancestorParse.
*/
export function getImplicitFromSelection(model: Model) {
const implicit: Dict<string> = {};

if (isUnitModel(model) && model.component.selection) {
for (const name of keys(model.component.selection)) {
const selCmpt = model.component.selection[name];
for (const proj of selCmpt.project.items) {
if (!proj.channel && accessPathDepth(proj.field) > 1) {
implicit[proj.field] = 'flatten';
}
}
}
}

return implicit;
}

export class ParseNode extends DataFlowNode {
private _parse: Parse;

Expand Down Expand Up @@ -89,121 +212,10 @@ export class ParseNode extends DataFlowNode {
return this.makeWithAncestors(parent, explicit, {}, ancestorParse);
}

public static makeImplicitFromFilterTransform(
parent: DataFlowNode,
transform: FilterTransform,
ancestorParse: AncestorParse
) {
const parse: Dict<string> = {};
forEachLeaf(transform.filter, filter => {
if (isFieldPredicate(filter)) {
// Automatically add a parse node for filters with filter objects
let val: string | number | boolean | DateTime = null;

// For EqualFilter, just use the equal property.
// For RangeFilter and OneOfFilter, all array members should have
// the same type, so we only use the first one.
if (isFieldEqualPredicate(filter)) {
val = filter.equal;
} else if (isFieldRangePredicate(filter)) {
val = filter.range[0];
} else if (isFieldOneOfPredicate(filter)) {
val = (filter.oneOf || filter['in'])[0];
} // else -- for filter expression, we can't infer anything
if (val) {
if (isDateTime(val)) {
parse[filter.field] = 'date';
} else if (isNumber(val)) {
parse[filter.field] = 'number';
} else if (isString(val)) {
parse[filter.field] = 'string';
}
}

if (filter.timeUnit) {
parse[filter.field] = 'date';
}
}
});

if (keys(parse).length === 0) {
return null;
}

return this.makeWithAncestors(parent, {}, parse, ancestorParse);
}

/**
* Creates a parse node for implicit parsing from a model and updates ancestorParse.
*/
public static makeImplicitFromEncoding(parent: DataFlowNode, model: Model, ancestorParse: AncestorParse) {
const implicit: Dict<string> = {};

function add(fieldDef: TypedFieldDef<string>) {
if (isTimeFormatFieldDef(fieldDef)) {
implicit[fieldDef.field] = 'date';
} else if (
fieldDef.type === 'quantitative' &&
isMinMaxOp(fieldDef.aggregate) // we need to parse numbers to support correct min and max
) {
implicit[fieldDef.field] = 'number';
} else if (accessPathDepth(fieldDef.field) > 1) {
// For non-date/non-number (strings and booleans), derive a flattened field for a referenced nested field.
// (Parsing numbers / dates already flattens numeric and temporal fields.)
if (!(fieldDef.field in implicit)) {
implicit[fieldDef.field] = 'flatten';
}
} else if (isScaleFieldDef(fieldDef) && isSortField(fieldDef.sort) && accessPathDepth(fieldDef.sort.field) > 1) {
// Flatten fields that we sort by but that are not otherwise flattened.
if (!(fieldDef.sort.field in implicit)) {
implicit[fieldDef.sort.field] = 'flatten';
}
}
}

if (isUnitModel(model) || isFacetModel(model)) {
// Parse encoded fields
model.forEachFieldDef((fieldDef, channel) => {
if (isTypedFieldDef(fieldDef)) {
add(fieldDef);
} else {
const mainChannel = getMainRangeChannel(channel);
const mainFieldDef = model.fieldDef(mainChannel as SingleDefChannel) as TypedFieldDef<string>;
add({
...fieldDef,
type: mainFieldDef.type
});
}
});
}

// Parse quantitative dimension fields of path marks as numbers so that we sort them correctly.
if (isUnitModel(model)) {
const {mark, markDef, encoding} = model;
if (
isPathMark(mark) &&
// No need to sort by dimension if we have a connected scatterplot (order channel is present)
!model.encoding.order
) {
const dimensionChannel = markDef.orient === 'horizontal' ? 'y' : 'x';
const dimensionChannelDef = encoding[dimensionChannel];
if (
isFieldDef(dimensionChannelDef) &&
dimensionChannelDef.type === 'quantitative' &&
!(dimensionChannelDef.field in implicit)
) {
implicit[dimensionChannelDef.field] = 'number';
}
}
}

return this.makeWithAncestors(parent, {}, implicit, ancestorParse);
}

/**
* Creates a parse node from "explicit" parse and "implicit" parse and updates ancestorParse.
*/
private static makeWithAncestors(
public static makeWithAncestors(
parent: DataFlowNode,
explicit: Parse,
implicit: Parse,
Expand Down
19 changes: 14 additions & 5 deletions src/compile/data/parse.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ import {
isJoinAggregate,
isLoess,
isLookup,
isRegression,
isPivot,
isRegression,
isSample,
isStack,
isTimeUnit,
Expand All @@ -43,7 +43,12 @@ import {FilterNode} from './filter';
import {FilterInvalidNode} from './filterinvalid';
import {FlattenTransformNode} from './flatten';
import {FoldTransformNode} from './fold';
import {ParseNode} from './formatparse';
import {
getImplicitFromEncoding,
getImplicitFromFilterTransform,
getImplicitFromSelection,
ParseNode
} from './formatparse';
import {GeoJSONNode} from './geojson';
import {GeoPointNode} from './geopoint';
import {GraticuleNode} from './graticule';
Expand All @@ -54,8 +59,8 @@ import {JoinAggregateTransformNode} from './joinaggregate';
import {makeJoinAggregateFromFacet} from './joinaggregatefacet';
import {LoessTransformNode} from './loess';
import {LookupNode} from './lookup';
import {RegressionTransformNode} from './regression';
import {PivotTransformNode} from './pivot';
import {RegressionTransformNode} from './regression';
import {SampleTransformNode} from './sample';
import {SequenceNode} from './sequence';
import {SourceNode} from './source';
Expand Down Expand Up @@ -145,7 +150,8 @@ export function parseTransformArray(head: DataFlowNode, model: Model, ancestorPa
transformNode = head = new CalculateNode(head, t);
derivedType = 'derived';
} else if (isFilter(t)) {
transformNode = head = ParseNode.makeImplicitFromFilterTransform(head, t, ancestorParse) || head;
const implicit = getImplicitFromFilterTransform(t);
transformNode = head = ParseNode.makeWithAncestors(head, {}, implicit, ancestorParse) || head;

head = new FilterNode(head, model, t.filter);
} else if (isBin(t)) {
Expand Down Expand Up @@ -321,7 +327,10 @@ export function parseData(model: Model): DataComponent {
head = parseTransformArray(head, model, ancestorParse);
}

head = ParseNode.makeImplicitFromEncoding(head, model, ancestorParse) || head;
// create parse nodes for fields that need to be parsed (or flattened) implicitly
const implicitSelection = getImplicitFromSelection(model);
const implicitEncoding = getImplicitFromEncoding(model);
head = ParseNode.makeWithAncestors(head, {}, {...implicitSelection, ...implicitEncoding}, ancestorParse) || head;

if (isUnitModel(model)) {
head = GeoJSONNode.parseAll(head, model);
Expand Down
4 changes: 2 additions & 2 deletions src/compile/selection/assemble.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {forEachSelection, MODIFY, SELECTION_DOMAIN, STORE, unitName, VL_SELECTIO
import {dateTimeExpr, isDateTime} from '../../datetime';
import {warn} from '../../log';
import {SelectionInit, SelectionInitInterval} from '../../selection';
import {accessPathWithDatum, keys, varName} from '../../util';
import {keys, varName} from '../../util';
import {VgData} from '../../vega.schema';
import {FacetModel} from '../facet';
import {LayerModel} from '../layer';
Expand Down Expand Up @@ -197,7 +197,7 @@ export function assembleSelectionScaleDomain(model: Model, domainRaw: SignalRef)
}
}

return {signal: accessPathWithDatum(field, name)};
return {signal: `${name}[${stringValue(field)}]`};
}

return {signal: 'null'};
Expand Down
4 changes: 2 additions & 2 deletions src/compile/selection/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
SelectionType,
SELECTION_ID
} from '../../selection';
import {accessPathWithDatum, Dict} from '../../util';
import {Dict} from '../../util';
import {FacetModel} from '../facet';
import {isFacetModel, Model} from '../model';
import {UnitModel} from '../unit';
Expand Down Expand Up @@ -92,7 +92,7 @@ export function unitName(model: Model, {escape} = {escape: true}) {
const {facet} = facetModel;
for (const channel of FACET_CHANNELS) {
if (facet[channel]) {
name += ` + '__facet_${channel}_' + (${accessPathWithDatum(facetModel.vgField(channel), 'facet')})`;
name += ` + '__facet_${channel}_' + (facet[${stringValue(facetModel.vgField(channel))}])`;
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/compile/selection/multi.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {Signal} from 'vega';
import {stringValue} from 'vega-util';
import {SelectionCompiler, SelectionComponent, TUPLE, unitName} from '.';
import {accessPathWithDatum} from '../../util';
import {UnitModel} from '../unit';
import {TUPLE_FIELDS} from './transforms/project';

Expand All @@ -14,9 +14,9 @@ export function singleOrMultiSignals(model: UnitModel, selCmpt: SelectionCompone
const fieldDef = model.fieldDef(p.channel);
// Binned fields should capture extents, for a range test against the raw field.
return fieldDef && fieldDef.bin
? `[${accessPathWithDatum(model.vgField(p.channel, {}), datum)}, ` +
`${accessPathWithDatum(model.vgField(p.channel, {binSuffix: 'end'}), datum)}]`
: `${accessPathWithDatum(p.field, datum)}`;
? `[${datum}[${stringValue(model.vgField(p.channel, {}))}], ` +
`${datum}[${stringValue(model.vgField(p.channel, {binSuffix: 'end'}))}]]`
: `${datum}[${stringValue(p.field)}]`;
})
.join(', ');

Expand Down
5 changes: 3 additions & 2 deletions src/compile/selection/transforms/inputs.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {stringValue} from 'vega-util';
import {TUPLE} from '..';
import {accessPathWithDatum, varName} from '../../../util';
import {varName} from '../../../util';
import {assembleInit} from '../assemble';
import nearest from './nearest';
import {TUPLE_FIELDS} from './project';
Expand Down Expand Up @@ -35,7 +36,7 @@ const inputBindings: TransformCompiler = {
? [
{
events: selCmpt.events,
update: `datum && item().mark.marktype !== 'group' ? ${accessPathWithDatum(p.field, datum)} : null`
update: `datum && item().mark.marktype !== 'group' ? ${datum}[${stringValue(p.field)}] : null`
}
]
: [],
Expand Down
Loading

0 comments on commit 88333ef

Please sign in to comment.