diff --git a/examples/compiled/bar_x_offset_without_x.png b/examples/compiled/bar_x_offset_without_x.png new file mode 100644 index 0000000000..dddf972e29 Binary files /dev/null and b/examples/compiled/bar_x_offset_without_x.png differ diff --git a/examples/compiled/bar_x_offset_without_x.svg b/examples/compiled/bar_x_offset_without_x.svg new file mode 100644 index 0000000000..339c464fe7 --- /dev/null +++ b/examples/compiled/bar_x_offset_without_x.svg @@ -0,0 +1 @@ +0.00.51.01.52.0valuexyzgroup \ No newline at end of file diff --git a/examples/compiled/bar_x_offset_without_x_broken.vg.json b/examples/compiled/bar_x_offset_without_x.vg.json similarity index 80% rename from examples/compiled/bar_x_offset_without_x_broken.vg.json rename to examples/compiled/bar_x_offset_without_x.vg.json index 7224919868..c3fb601dde 100644 --- a/examples/compiled/bar_x_offset_without_x_broken.vg.json +++ b/examples/compiled/bar_x_offset_without_x.vg.json @@ -1,8 +1,9 @@ { "$schema": "https://vega.github.io/schema/vega/v5.json", - "description": "xOffset without x will be replaced as x", + "description": "xOffset without x", "background": "white", "padding": 5, + "width": 20, "height": 200, "style": "cell", "data": [ @@ -39,13 +40,6 @@ ] } ], - "signals": [ - {"name": "x_step", "value": 20}, - { - "name": "width", - "update": "bandspace(domain('x').length, 0.1, 0.05) * x_step" - } - ], "marks": [ { "name": "marks", @@ -59,8 +53,12 @@ "description": { "signal": "\"value: \" + (format(datum[\"value\"], \"\")) + \"; group: \" + (isValid(datum[\"group\"]) ? datum[\"group\"] : \"\"+datum[\"group\"])" }, - "x": {"scale": "x", "field": "group"}, - "width": {"signal": "max(0.25, bandwidth('x'))"}, + "x": { + "signal": "width", + "mult": 0.5, + "offset": {"scale": "xOffset", "field": "group"} + }, + "width": {"signal": "max(0.25, bandwidth('xOffset'))"}, "y": {"scale": "y", "field": "value_end"}, "y2": {"scale": "y", "field": "value_start"} } @@ -68,14 +66,6 @@ } ], "scales": [ - { - "name": "x", - "type": "band", - "domain": {"data": "data_0", "field": "group", "sort": true}, - "range": {"step": {"signal": "x_step"}}, - "paddingInner": 0.1, - "paddingOuter": 0.05 - }, { "name": "y", "type": "linear", @@ -84,6 +74,12 @@ "nice": true, "zero": true }, + { + "name": "xOffset", + "type": "band", + "domain": {"data": "data_0", "field": "group", "sort": true}, + "range": [{"signal": "-width/2"}, {"signal": "width/2"}] + }, { "name": "color", "type": "ordinal", @@ -95,7 +91,6 @@ { "scale": "y", "orient": "left", - "gridScale": "x", "grid": true, "tickCount": {"signal": "ceil(height/40)"}, "domain": false, @@ -106,16 +101,6 @@ "ticks": false, "zindex": 0 }, - { - "scale": "x", - "orient": "bottom", - "grid": false, - "title": "group", - "labelAlign": "right", - "labelAngle": 270, - "labelBaseline": "middle", - "zindex": 0 - }, { "scale": "y", "orient": "left", diff --git a/examples/compiled/bar_x_offset_without_x_broken.png b/examples/compiled/bar_x_offset_without_x_broken.png deleted file mode 100644 index 78a74f2ff1..0000000000 Binary files a/examples/compiled/bar_x_offset_without_x_broken.png and /dev/null differ diff --git a/examples/compiled/bar_x_offset_without_x_broken.svg b/examples/compiled/bar_x_offset_without_x_broken.svg deleted file mode 100644 index c265a3fbdf..0000000000 --- a/examples/compiled/bar_x_offset_without_x_broken.svg +++ /dev/null @@ -1 +0,0 @@ -xyzgroup0.00.51.01.52.0valuexyzgroup \ No newline at end of file diff --git a/examples/specs/bar_x_offset_without_x_broken.vl.json b/examples/specs/bar_x_offset_without_x.vl.json similarity index 92% rename from examples/specs/bar_x_offset_without_x_broken.vl.json rename to examples/specs/bar_x_offset_without_x.vl.json index 8a702c5fd2..d106c6c338 100644 --- a/examples/specs/bar_x_offset_without_x_broken.vl.json +++ b/examples/specs/bar_x_offset_without_x.vl.json @@ -1,6 +1,6 @@ { "$schema": "https://vega.github.io/schema/vega-lite/v5.json", - "description": "xOffset without x will be replaced as x", + "description": "xOffset without x", "data": { "values": [ {"category":"A", "group": "x", "value":0.1}, diff --git a/src/compile/mark/encode/position-rect.ts b/src/compile/mark/encode/position-rect.ts index afbd417d48..b2229b1433 100644 --- a/src/compile/mark/encode/position-rect.ts +++ b/src/compile/mark/encode/position-rect.ts @@ -166,7 +166,14 @@ function positionAndSize( const hasSizeFromMarkOrEncoding = !!sizeMixins; // Otherwise, apply default value - const bandSize = getBandSize({channel, fieldDef, markDef, config, scaleType: scale?.get('type'), useVlSizeChannel}); + const bandSize = getBandSize({ + channel, + fieldDef, + markDef, + config, + scaleType: (scale || offsetScale)?.get('type'), + useVlSizeChannel + }); sizeMixins = sizeMixins || { [vgSizeChannel]: defaultSizeRef( @@ -190,7 +197,9 @@ function positionAndSize( */ const defaultBandAlign = - scale?.get('type') === 'band' && isRelativeBandSize(bandSize) && !hasSizeFromMarkOrEncoding ? 'top' : 'middle'; + (scale || offsetScale)?.get('type') === 'band' && isRelativeBandSize(bandSize) && !hasSizeFromMarkOrEncoding + ? 'top' + : 'middle'; const vgChannel = vgAlignedPositionChannel(channel, markDef, config, defaultBandAlign); const center = vgChannel === 'xc' || vgChannel === 'yc'; diff --git a/src/compile/scale/parse.ts b/src/compile/scale/parse.ts index a2334bcf7a..e6baf799af 100644 --- a/src/compile/scale/parse.ts +++ b/src/compile/scale/parse.ts @@ -1,7 +1,6 @@ -import {getMainChannelFromOffsetChannel, isXorYOffset, ScaleChannel, SCALE_CHANNELS, SHAPE} from '../../channel'; +import {ScaleChannel, SCALE_CHANNELS, SHAPE} from '../../channel'; import {getFieldOrDatumDef, ScaleDatumDef, TypedFieldDef} from '../../channeldef'; import {channelHasNestedOffsetScale} from '../../encoding'; -import * as log from '../../log'; import {GEOSHAPE} from '../../mark'; import { NON_TYPE_DOMAIN_RANGE_VEGA_SCALE_PROPERTIES, @@ -56,17 +55,6 @@ function parseUnitScaleCore(model: UnitModel): ScaleComponentIndex { } let specifiedScale = fieldOrDatumDef && fieldOrDatumDef['scale']; - if (isXorYOffset(channel)) { - const mainChannel = getMainChannelFromOffsetChannel(channel); - if (!channelHasNestedOffsetScale(encoding, mainChannel)) { - // Don't generate scale when the offset encoding shouldn't yield a nested scale - if (specifiedScale) { - log.warn(log.message.offsetEncodingScaleIgnored(channel)); - } - continue; - } - } - if (fieldOrDatumDef && specifiedScale !== null && specifiedScale !== false) { specifiedScale ??= {}; const hasNestedOffsetScale = channelHasNestedOffsetScale(encoding, channel); diff --git a/src/compile/scale/range.ts b/src/compile/scale/range.ts index a25d382470..e114d5625e 100644 --- a/src/compile/scale/range.ts +++ b/src/compile/scale/range.ts @@ -221,11 +221,39 @@ function parseScheme(scheme: Scheme | SignalRef): RangeScheme { return {scheme}; } +function fullWidthOrHeightRange( + channel: 'x' | 'y', + model: UnitModel, + scaleType: ScaleType, + {center}: {center?: boolean} = {} +) { + // If step is null, use zero to width or height. + // Note that we use SignalRefWrapper to account for potential merges and renames. + const sizeType = getSizeChannel(channel); + const sizeSignal = model.getName(sizeType); + const getSignalName = model.getSignalName.bind(model); + + if (channel === Y && hasContinuousDomain(scaleType)) { + // For y continuous scale, we have to start from the height as the bottom part has the max value. + return center + ? [ + SignalRefWrapper.fromName(name => `${getSignalName(name)}/2`, sizeSignal), + SignalRefWrapper.fromName(name => `-${getSignalName(name)}/2`, sizeSignal) + ] + : [SignalRefWrapper.fromName(getSignalName, sizeSignal), 0]; + } else { + return center + ? [ + SignalRefWrapper.fromName(name => `-${getSignalName(name)}/2`, sizeSignal), + SignalRefWrapper.fromName(name => `${getSignalName(name)}/2`, sizeSignal) + ] + : [0, SignalRefWrapper.fromName(getSignalName, sizeSignal)]; + } +} + function defaultRange(channel: ScaleChannel, model: UnitModel): VgRange { const {size, config, mark, encoding} = model; - const getSignalName = model.getSignalName.bind(model); - const {type} = getFieldOrDatumDef(encoding[channel]) as ScaleFieldDef | ScaleDatumDef; const mergedScaleCmpt = model.getScaleComponent(channel); @@ -245,18 +273,7 @@ function defaultRange(channel: ScaleChannel, model: UnitModel): VgRange { } } - // If step is null, use zero to width or height. - // Note that we use SignalRefWrapper to account for potential merges and renames. - - const sizeType = getSizeChannel(channel); - const sizeSignal = model.getName(sizeType); - - if (channel === Y && hasContinuousDomain(scaleType)) { - // For y continuous scale, we have to start from the height as the bottom part has the max value. - return [SignalRefWrapper.fromName(getSignalName, sizeSignal), 0]; - } else { - return [0, SignalRefWrapper.fromName(getSignalName, sizeSignal)]; - } + return fullWidthOrHeightRange(channel, model, scaleType); } case XOFFSET: @@ -374,6 +391,11 @@ function getOffsetStep(step: Step, offsetScaleType: ScaleType) { function getOffsetRange(channel: string, model: UnitModel, offsetScaleType: ScaleType): VgRange { const positionChannel = channel === XOFFSET ? 'x' : 'y'; const positionScaleCmpt = model.getScaleComponent(positionChannel); + + if (!positionScaleCmpt) { + return fullWidthOrHeightRange(positionChannel, model, offsetScaleType, {center: true}); + } + const positionScaleType = positionScaleCmpt.get('type'); const positionScaleName = model.scaleName(positionChannel); diff --git a/src/encoding.ts b/src/encoding.ts index 7052be9662..0dafbc975e 100644 --- a/src/encoding.ts +++ b/src/encoding.ts @@ -558,10 +558,6 @@ export function initEncoding( continue; } } - } else { - // no x/y, replace it with main channel - channel = mainChannel; - log.warn(log.message.replaceOffsetWithMainChannel(mainChannel)); } } diff --git a/src/log/message.ts b/src/log/message.ts index adb059e2fe..4f2cfd11d3 100644 --- a/src/log/message.ts +++ b/src/log/message.ts @@ -154,10 +154,6 @@ export function offsetNestedInsideContinuousPositionScaleDropped(mainChannel: Po return `${mainChannel}Offset dropped because ${mainChannel} is continuous`; } -export function replaceOffsetWithMainChannel(mainChannel: PositionScaleChannel) { - return `There is no ${mainChannel} encoding. Replacing ${mainChannel}Offset encoding as ${mainChannel}.`; -} - export function primitiveChannelDef( channel: ExtendedChannel, type: 'string' | 'number' | 'boolean', diff --git a/src/stack.ts b/src/stack.ts index db05480795..00e7bb3b38 100644 --- a/src/stack.ts +++ b/src/stack.ts @@ -176,16 +176,16 @@ export function stack(m: Mark | MarkDef, encoding: Encoding): StackPrope groupbyChannels.push(dimensionChannel); groupbyFields.add(dimensionField); } + } - const dimensionOffsetChannel = dimensionChannel === 'x' ? 'xOffset' : 'yOffset'; - const dimensionOffsetDef = encoding[dimensionOffsetChannel]; - const dimensionOffsetField = isFieldDef(dimensionOffsetDef) ? vgField(dimensionOffsetDef, {}) : undefined; + const dimensionOffsetChannel = dimensionChannel === 'x' ? 'xOffset' : 'yOffset'; + const dimensionOffsetDef = encoding[dimensionOffsetChannel]; + const dimensionOffsetField = isFieldDef(dimensionOffsetDef) ? vgField(dimensionOffsetDef, {}) : undefined; - if (dimensionOffsetField && dimensionOffsetField !== stackedField) { - // avoid grouping by the stacked field - groupbyChannels.push(dimensionOffsetChannel); - groupbyFields.add(dimensionOffsetField); - } + if (dimensionOffsetField && dimensionOffsetField !== stackedField) { + // avoid grouping by the stacked field + groupbyChannels.push(dimensionOffsetChannel); + groupbyFields.add(dimensionOffsetField); } // If the dimension has offset, don't stack anymore diff --git a/test/compile/mark/encode/position-rect.test.ts b/test/compile/mark/encode/position-rect.test.ts index a053a2d4d3..f042b1f725 100644 --- a/test/compile/mark/encode/position-rect.test.ts +++ b/test/compile/mark/encode/position-rect.test.ts @@ -29,6 +29,33 @@ describe('compile/mark/encode/position-rect', () => { }); }); + it('produces correct x-mixins for xOffset without x', () => { + const model = parseUnitModelWithScaleAndLayoutSize({ + data: {values: []}, + mark: 'bar', + encoding: { + xOffset: { + field: 'a', + type: 'nominal' + }, + y: { + field: 'b', + type: 'quantitative' + } + } + }); + + const props = rectPosition(model, 'x'); + expect(props.x).toEqual({ + signal: 'width', + mult: 0.5, + offset: {scale: 'xOffset', field: 'a'} + }); + expect(props.width).toEqual({ + signal: `max(0.25, bandwidth('xOffset'))` + }); + }); + it('produces correct x-mixins for timeUnit with bandPosition = 0', () => { const model = parseUnitModelWithScaleAndLayoutSize({ data: {values: []}, diff --git a/test/compile/scale/range.test.ts b/test/compile/scale/range.test.ts index 36cb40bec6..2e92b6866e 100644 --- a/test/compile/scale/range.test.ts +++ b/test/compile/scale/range.test.ts @@ -132,6 +132,48 @@ describe('compile/scale', () => { ); }); + it('should return [-width/2, width/2] for xOffset without x', () => { + const model = parseUnitModelWithScaleExceptRange({ + mark: 'bar', + encoding: { + xOffset: {field: 'xSub', type: 'nominal'}, + y: {field: 'y', type: 'nominal'} + } + }); + + expect(parseRangeForChannel('xOffset', model)).toEqual( + makeImplicit([ + { + signal: '-width/2' + }, + { + signal: 'width/2' + } + ]) + ); + }); + + it('should return [-height/2, height/2] for yOffset without y', () => { + const model = parseUnitModelWithScaleExceptRange({ + mark: 'bar', + encoding: { + yOffset: {field: 'xSub', type: 'nominal'}, + x: {field: 'y', type: 'nominal'} + } + }); + + expect(parseRangeForChannel('yOffset', model)).toEqual( + makeImplicit([ + { + signal: '-height/2' + }, + { + signal: 'height/2' + } + ]) + ); + }); + it('should return padded duration range when there is a nested offset with year time scale and default padding', () => { const model = parseUnitModelWithScaleExceptRange({ mark: 'bar', diff --git a/test/encoding.test.ts b/test/encoding.test.ts index 36cd19c454..847bda7c6d 100644 --- a/test/encoding.test.ts +++ b/test/encoding.test.ts @@ -87,25 +87,6 @@ describe('encoding', () => { }) ); - it( - 'replaces xOffset with x if there is no x', - log.wrap(logger => { - const encoding = initEncoding( - { - xOffset: {field: 'a', type: 'quantitative'} - }, - 'point', - false, - defaultConfig - ); - - expect(encoding).toEqual({ - x: {field: 'a', type: 'quantitative'} - }); - expect(logger.warns[0]).toEqual(log.message.replaceOffsetWithMainChannel('x')); - }) - ); - it( 'drops xOffset if x is continuous', log.wrap(logger => { @@ -125,6 +106,7 @@ describe('encoding', () => { expect(logger.warns[0]).toEqual(log.message.offsetNestedInsideContinuousPositionScaleDropped('x')); }) ); + it('does not drop xOffset if x is time with timeUnit', () => { const encoding = initEncoding( { diff --git a/test/stack.test.ts b/test/stack.test.ts index 455748d07d..d215d92250 100644 --- a/test/stack.test.ts +++ b/test/stack.test.ts @@ -1,5 +1,5 @@ import {NonArgAggregateOp} from '../src/aggregate'; -import {DETAIL, X, Y} from '../src/channel'; +import {DETAIL, X, Y, YOFFSET} from '../src/channel'; import * as log from '../src/log'; import {ARC, AREA, BAR, PRIMITIVE_MARKS, RECT} from '../src/mark'; import {ScaleType} from '../src/scale'; @@ -327,6 +327,41 @@ describe('stack', () => { } }); + it('should be correct for grouped bar', () => { + for (const stackableMark of [BAR, AREA]) { + const spec: TopLevel = { + data: {url: 'data/barley.json'}, + mark: stackableMark, + encoding: { + x: {field: 'yield', type: 'quantitative'}, + y: {field: 'variety', type: 'nominal'}, + yOffset: {field: 'site', type: 'nominal'}, + color: {field: 'site', type: 'nominal'} + } + }; + const _stack = stack(spec.mark, spec.encoding); + expect(_stack.fieldChannel).toBe(X); + expect(_stack.groupbyChannels).toEqual([Y, YOFFSET]); + } + }); + + it('should be correct for grouped bar without nesting', () => { + for (const stackableMark of [BAR, AREA]) { + const spec: TopLevel = { + data: {url: 'data/barley.json'}, + mark: stackableMark, + encoding: { + x: {field: 'yield', type: 'quantitative'}, + yOffset: {field: 'site', type: 'nominal'}, + color: {field: 'site', type: 'nominal'} + } + }; + const _stack = stack(spec.mark, spec.encoding); + expect(_stack.fieldChannel).toBe(X); + expect(_stack.groupbyChannels).toEqual([YOFFSET]); + } + }); + it('should stack even for two plain quantitatives with the orient on the axes', () => { for (const mark of [BAR, AREA]) { const spec: TopLevel = {