From 52d132dbf0003f0522d357b3a10c7ca134335361 Mon Sep 17 00:00:00 2001 From: Dominik Moritz Date: Sat, 14 Jul 2018 20:12:32 -0700 Subject: [PATCH] Correctly generate `as` for nested fields. Note that in Vega `as` cannot be nested so you have to provide a field name, not a field accessor. Fixes #3744. --- examples/specs/test_aggregate_nested.vl.json | 36 ++++++++++++++++++++ src/compile/data/aggregate.ts | 18 +++++----- src/compile/data/bin.ts | 6 ++-- src/compile/data/calculate.ts | 13 ++++--- src/compile/data/facet.ts | 4 +-- src/compile/data/stack.ts | 15 ++++---- src/compile/data/timeunit.ts | 4 +-- src/compile/data/window.ts | 2 +- src/compile/facet.ts | 11 ++++-- src/compile/model.ts | 12 ++++++- src/encoding.ts | 5 +-- src/fielddef.ts | 24 ++++++++++--- src/util.ts | 3 +- 13 files changed, 113 insertions(+), 40 deletions(-) create mode 100644 examples/specs/test_aggregate_nested.vl.json diff --git a/examples/specs/test_aggregate_nested.vl.json b/examples/specs/test_aggregate_nested.vl.json new file mode 100644 index 0000000000..aef605f48b --- /dev/null +++ b/examples/specs/test_aggregate_nested.vl.json @@ -0,0 +1,36 @@ +{ + + "$schema": "https://vega.github.io/schema/vega-lite/v2.json", + "mark": "bar", + "encoding": { + "y": { "field": "properties.variety", "type": "nominal"}, + "x": { + "aggregate": "sum", + "field": "properties.yield", + "type": "quantitative" + }, + "color": {"title": "site", "field": "properties.site", "type": "nominal"} + }, + "data": { + "values": { + "features": [ + { + "properties": { + "variety": "Manchuria", + "yield": 27, + "site": "University Farm" + } + }, + { + "properties": { + "variety": "Wisconsin No. 38", + "yield": 29.33333, + "site": "Duluth" + } + } + ], + "type": "FeatureCollection" + }, + "format": {"type": "json", "property": "features"} + } +} diff --git a/src/compile/data/aggregate.ts b/src/compile/data/aggregate.ts index 3bea48dae1..be24267367 100644 --- a/src/compile/data/aggregate.ts +++ b/src/compile/data/aggregate.ts @@ -1,10 +1,10 @@ import {AggregateOp} from 'vega'; import {isBinning} from '../../bin'; import {Channel, isScaleChannel} from '../../channel'; -import {FieldDef, vgField} from '../../fielddef'; +import {FieldDef, vgField, vgFieldName} from '../../fielddef'; import * as log from '../../log'; import {AggregateTransform} from '../../transform'; -import {Dict, differ, duplicate, keys, StringSet} from '../../util'; +import {Dict, differ, duplicate, keys, replacePathInField, StringSet} from '../../util'; import {VgAggregateTransform} from '../../vega.schema'; import {binRequiresRange} from '../common'; import {UnitModel} from './../unit'; @@ -81,15 +81,15 @@ export class AggregateNode extends DataFlowNode { if (aggregate) { if (aggregate === 'count') { meas['*'] = meas['*'] || {}; - meas['*']['count'] = vgField(fieldDef); + meas['*']['count'] = vgFieldName(fieldDef); } else { meas[field] = meas[field] || {}; - meas[field][aggregate] = vgField(fieldDef); + meas[field][aggregate] = vgFieldName(fieldDef); // For scale channel with domain === 'unaggregated', add min/max so we can use their union as unaggregated domain if (isScaleChannel(channel) && model.scaleDomain(channel) === 'unaggregated') { - meas[field]['min'] = vgField({field, aggregate: 'min'}); - meas[field]['max'] = vgField({field, aggregate: 'max'}); + meas[field]['min'] = vgFieldName({field, aggregate: 'min'}); + meas[field]['max'] = vgFieldName({field, aggregate: 'max'}); } } } else { @@ -113,10 +113,10 @@ export class AggregateNode extends DataFlowNode { if (op) { if (op === 'count') { meas['*'] = meas['*'] || {}; - meas['*']['count'] = as || vgField(s); + meas['*']['count'] = as || vgFieldName(s); } else { meas[field] = meas[field] || {}; - meas[field][op] = as || vgField(s); + meas[field][op] = as || vgFieldName(s); } } } @@ -175,7 +175,7 @@ export class AggregateNode extends DataFlowNode { for (const op of keys(this.measures[field])) { as.push(this.measures[field][op]); ops.push(op); - fields.push(field); + fields.push(replacePathInField(field)); } } diff --git a/src/compile/data/bin.ts b/src/compile/data/bin.ts index 030ad8b3cb..4a20632f4f 100644 --- a/src/compile/data/bin.ts +++ b/src/compile/data/bin.ts @@ -2,7 +2,7 @@ import {isString} from 'vega-util'; import {BinParams, binToString, isBinning} from '../../bin'; import {Channel} from '../../channel'; import {Config} from '../../config'; -import {FieldDef, normalizeBin, vgField} from '../../fielddef'; +import {FieldDef, normalizeBin, vgField, vgFieldName} from '../../fielddef'; import {BinTransform} from '../../transform'; import {Dict, duplicate, flatten, keys, vals} from '../../util'; import {VgBinTransform, VgTransform} from '../../vega.schema'; @@ -20,7 +20,7 @@ function rangeFormula(model: ModelWithField, fieldDef: FieldDef, channel const endField = vgField(fieldDef, {expr: 'datum', binSuffix: 'end'}); return { - formulaAs: vgField(fieldDef, {binSuffix: 'range'}), + formulaAs: vgFieldName(fieldDef, {binSuffix: 'range'}), formula: binFormatExpression(startField, endField, guide.format, config) }; } @@ -48,7 +48,7 @@ function createBinComponent(t: FieldDef | BinTransform, bin: boolean | B if (isBinTransform(t)) { as = isString(t.as) ? [t.as, `${t.as}_end`] : [t.as[0], t.as[1]]; } else { - as = [vgField(t, {}), vgField(t, {binSuffix: 'end'})]; + as = [vgFieldName(t, {}), vgFieldName(t, {binSuffix: 'end'})]; } const normalizedBin = normalizeBin(bin, undefined) || {}; diff --git a/src/compile/data/calculate.ts b/src/compile/data/calculate.ts index 718e898c06..a5b5f8a78e 100644 --- a/src/compile/data/calculate.ts +++ b/src/compile/data/calculate.ts @@ -1,5 +1,5 @@ import {DateTime} from '../../datetime'; -import {FieldDef, isScaleFieldDef, vgField} from '../../fielddef'; +import {FieldDef, isScaleFieldDef, vgField, vgFieldName} from '../../fielddef'; import {fieldFilterExpression} from '../../predicate'; import {isSortArray} from '../../sort'; import {duplicate} from '../../util'; @@ -40,7 +40,7 @@ export class CalculateNode extends DataFlowNode { parent = new CalculateNode(parent, { calculate, - as: sortArrayIndexField(fieldDef, channel) + as: sortArrayIndexField(fieldDef, channel, undefined, true) }); } }); @@ -62,6 +62,11 @@ export class CalculateNode extends DataFlowNode { } } -export function sortArrayIndexField(fieldDef: FieldDef, channel: SingleDefChannel, expr?: 'datum') { - return vgField(fieldDef, {prefix: channel, suffix: 'sort_index', expr}); +export function sortArrayIndexField( + fieldDef: FieldDef, + channel: SingleDefChannel, + expr?: 'datum', + forAs = false +) { + return (forAs ? vgFieldName : vgField)(fieldDef, {prefix: channel, suffix: 'sort_index', expr}); } diff --git a/src/compile/data/facet.ts b/src/compile/data/facet.ts index 052a1a5d7c..b5617f8810 100644 --- a/src/compile/data/facet.ts +++ b/src/compile/data/facet.ts @@ -2,7 +2,7 @@ import {AggregateOp} from 'vega'; import {isArray} from 'vega-util'; import {isBinning} from '../../bin'; import {COLUMN, ROW, ScaleChannel} from '../../channel'; -import {vgField} from '../../fielddef'; +import {vgField, vgFieldName} from '../../fielddef'; import * as log from '../../log'; import {hasDiscreteDomain} from '../../scale'; import {EncodingSortField, isSortField} from '../../sort'; @@ -133,7 +133,7 @@ export class FacetNode extends DataFlowNode { const {op, field} = sortField; fields.push(field); ops.push(op); - as.push(vgField(sortField)); + as.push(vgFieldName(sortField)); } else if (sortIndexField) { fields.push(sortIndexField); ops.push('max'); diff --git a/src/compile/data/stack.ts b/src/compile/data/stack.ts index 61d5873884..dfcc375ea7 100644 --- a/src/compile/data/stack.ts +++ b/src/compile/data/stack.ts @@ -1,5 +1,5 @@ import {isArray, isString} from 'vega-util'; -import {FieldDef, isFieldDef, vgField} from '../../fielddef'; +import {FieldDef, isFieldDef, vgField, vgFieldName} from '../../fielddef'; import {StackOffset} from '../../stack'; import {StackTransform} from '../../transform'; import {duplicate} from '../../util'; @@ -147,19 +147,19 @@ export class StackNode extends DataFlowNode { {field: [], order: []} ); } - // Refactored to add "as" in the make phase so that we can get producedFields - // from the as property - const field = model.vgField(stackProperties.fieldChannel); return new StackNode(parent, { dimensionFieldDef, - stackField: field, + stackField: model.vgField(stackProperties.fieldChannel), facetby: [], stackby, sort, offset: stackProperties.offset, impute: stackProperties.impute, - as: [field + '_start', field + '_end'] + as: [ + model.vgFieldName(stackProperties.fieldChannel, {suffix: 'start'}), + model.vgFieldName(stackProperties.fieldChannel, {suffix: 'end'}) + ] }); } @@ -218,6 +218,7 @@ export class StackNode extends DataFlowNode { // Impute if (impute && dimensionFieldDef) { const dimensionField = dimensionFieldDef ? vgField(dimensionFieldDef, {binSuffix: 'mid'}) : undefined; + const dimensionAs = dimensionFieldDef ? vgFieldName(dimensionFieldDef, {binSuffix: 'mid'}) : undefined; if (dimensionFieldDef.bin) { // As we can only impute one field at a time, we need to calculate @@ -230,7 +231,7 @@ export class StackNode extends DataFlowNode { '+' + vgField(dimensionFieldDef, {expr: 'datum', binSuffix: 'end'}) + ')/2', - as: dimensionField + as: dimensionAs }); } diff --git a/src/compile/data/timeunit.ts b/src/compile/data/timeunit.ts index 5401488219..7e4b337643 100644 --- a/src/compile/data/timeunit.ts +++ b/src/compile/data/timeunit.ts @@ -1,4 +1,4 @@ -import {vgField} from '../../fielddef'; +import {vgFieldName} from '../../fielddef'; import {fieldExpr, TimeUnit} from '../../timeunit'; import {TimeUnitTransform} from '../../transform'; import {Dict, duplicate, keys, vals} from '../../util'; @@ -25,7 +25,7 @@ export class TimeUnitNode extends DataFlowNode { const formula = model.reduceFieldDef( (timeUnitComponent: TimeUnitComponent, fieldDef) => { if (fieldDef.timeUnit) { - const f = vgField(fieldDef); + const f = vgFieldName(fieldDef); timeUnitComponent[f] = { as: f, timeUnit: fieldDef.timeUnit, diff --git a/src/compile/data/window.ts b/src/compile/data/window.ts index 8d6bd7e930..172964a750 100644 --- a/src/compile/data/window.ts +++ b/src/compile/data/window.ts @@ -25,7 +25,7 @@ export class WindowTransformNode extends DataFlowNode { { op, field, - as: facetSortFieldName(fieldDef, fieldDef.sort) + as: facetSortFieldName(fieldDef, fieldDef.sort, undefined, true) } ], groupby: [vgField(fieldDef)], diff --git a/src/compile/facet.ts b/src/compile/facet.ts index 2725a548a3..337e23e6b0 100644 --- a/src/compile/facet.ts +++ b/src/compile/facet.ts @@ -4,7 +4,7 @@ import {Channel, COLUMN, ROW, ScaleChannel} from '../channel'; import {Config} from '../config'; import {reduce} from '../encoding'; import {FacetFieldDef, FacetMapping} from '../facet'; -import {FieldDef, normalize, title as fieldDefTitle, vgField} from '../fielddef'; +import {FieldDef, normalize, title as fieldDefTitle, vgField, vgFieldName} from '../fielddef'; import * as log from '../log'; import {hasDiscreteDomain} from '../scale'; import {EncodingSortField, isSortField, SortOrder} from '../sort'; @@ -23,8 +23,13 @@ import {RepeaterValue, replaceRepeaterInFacet} from './repeater'; import {parseGuideResolve} from './resolve'; import {assembleDomain, getFieldFromDomain} from './scale/domain'; -export function facetSortFieldName(fieldDef: FacetFieldDef, sort: EncodingSortField, expr?: 'datum') { - return vgField(sort, {expr, suffix: `by_${vgField(fieldDef)}`}); +export function facetSortFieldName( + fieldDef: FacetFieldDef, + sort: EncodingSortField, + expr?: 'datum', + forAs = false +) { + return (forAs ? vgFieldName : vgField)(sort, {expr, suffix: `by_${vgField(fieldDef)}`}); } export class FacetModel extends ModelWithField { diff --git a/src/compile/model.ts b/src/compile/model.ts index cec470238c..518514728e 100644 --- a/src/compile/model.ts +++ b/src/compile/model.ts @@ -5,7 +5,7 @@ import {Channel, isChannel, isScaleChannel, ScaleChannel, SingleDefChannel} from import {Config} from '../config'; import {Data, DataSourceType} from '../data'; import {forEach, reduce} from '../encoding'; -import {ChannelDef, FieldDef, FieldRefOption, getFieldDef, vgField} from '../fielddef'; +import {ChannelDef, FieldDef, FieldRefOption, getFieldDef, vgField, vgFieldName} from '../fielddef'; import * as log from '../log'; import {Resolve} from '../resolve'; import {hasDiscreteDomain} from '../scale'; @@ -633,6 +633,16 @@ export abstract class ModelWithField extends Model { return vgField(fieldDef, opt); } + public vgFieldName(channel: SingleDefChannel, opt: FieldRefOption = {}) { + const fieldDef = this.fieldDef(channel); + + if (!fieldDef) { + return undefined; + } + + return vgFieldName(fieldDef, opt); + } + protected abstract getMapping(): {[key in Channel]?: any}; public reduceFieldDef(f: (acc: U, fd: FieldDef, c: Channel) => U, init: T, t?: any) { diff --git a/src/encoding.ts b/src/encoding.ts index dee6fb0355..d6ecf07b89 100644 --- a/src/encoding.ts +++ b/src/encoding.ts @@ -24,7 +24,8 @@ import { title, ValueDef, ValueDefWithCondition, - vgField + vgField, + vgFieldName } from './fielddef'; import * as log from './log'; import {Mark} from './mark'; @@ -203,7 +204,7 @@ export function extractTransformsFromEncoding(oldEncoding: Encoding, con forEach(oldEncoding, (channelDef, channel) => { if (isFieldDef(channelDef)) { - const transformedField = vgField(channelDef); + const transformedField = vgFieldName(channelDef); if (channelDef.aggregate && isAggregateOp(channelDef.aggregate)) { aggregate.push({ op: channelDef.aggregate, diff --git a/src/fielddef.ts b/src/fielddef.ts index 42f4b6f9c6..ebbaa9731f 100644 --- a/src/fielddef.ts +++ b/src/fielddef.ts @@ -339,15 +339,15 @@ export function isScaleFieldDef(channelDef: ChannelDef): channelDef is Sca } export interface FieldRefOption { - /** exclude bin, aggregate, timeUnit */ + /** Exclude bin, aggregate, timeUnit */ nofn?: boolean; /** Wrap the field with datum or parent (e.g., datum['...'] for Vega Expression */ expr?: 'datum' | 'parent'; - /** prepend fn with custom function prefix */ + /** Prepend fn with custom function prefix */ prefix?: string; - /** append suffix to the field ref for bin (default='start') */ + /** Append suffix to the field ref for bin (default='start') */ binSuffix?: 'end' | 'range' | 'mid'; - /** append suffix to the field ref (general) */ + /** Append suffix to the field ref (general) */ suffix?: string; } @@ -357,7 +357,10 @@ function isOpFieldDef( return !!fieldDef['op']; } -export function vgField( +/** + * Use when Vega expects a field name for example as the output of a transform. + */ +export function vgFieldName( fieldDef: FieldDefBase | WindowFieldDef | AggregatedFieldDef, opt: FieldRefOption = {} ): string { @@ -396,6 +399,17 @@ export function vgField( field = `${prefix}_${field}`; } + return field; +} + +/** + * Get a vega field reference from a Vega-Lite field def. Use when Vega expects a field that can be nested. + */ +export function vgField( + fieldDef: FieldDefBase | WindowFieldDef | AggregatedFieldDef, + opt: FieldRefOption = {} +): string { + const field = vgFieldName(fieldDef, opt); if (opt.expr) { // Expression to access flattened field. No need to escape dots. return flatAccessWithDatum(field, opt.expr); diff --git a/src/util.ts b/src/util.ts index bd4ad1239a..c518063419 100644 --- a/src/util.ts +++ b/src/util.ts @@ -297,7 +297,8 @@ export function accessPathWithDatum(path: string, datum = 'datum') { } /** - * Return access with datum to the falttened field. + * Return access with datum to the flattened field. + * * @param path The field name. * @param datum The string to use for `datum`. */