Skip to content

Commit

Permalink
fix: orient logic for when bar with x=T + simplify logic (#8739)
Browse files Browse the repository at this point in the history
Co-authored-by: GitHub Actions Bot <vega-actions-bot@users.noreply.github.com>
  • Loading branch information
kanitw and GitHub Actions Bot authored Feb 25, 2023
1 parent 27b320b commit ea4aa64
Show file tree
Hide file tree
Showing 7 changed files with 115 additions and 26 deletions.
Binary file added examples/compiled/bar_1d_temporal.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions examples/compiled/bar_1d_temporal.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
76 changes: 76 additions & 0 deletions examples/compiled/bar_1d_temporal.vg.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
{
"$schema": "https://vega.github.io/schema/vega/v5.json",
"background": "white",
"padding": 5,
"width": 200,
"height": 20,
"style": "cell",
"data": [
{
"name": "source_0",
"url": "data/cars.json",
"format": {"type": "json", "parse": {"Year": "date"}},
"transform": [
{
"type": "filter",
"expr": "(isDate(datum[\"Year\"]) || (isValid(datum[\"Year\"]) && isFinite(+datum[\"Year\"])))"
}
]
}
],
"marks": [
{
"name": "marks",
"type": "rect",
"style": ["bar"],
"from": {"data": "source_0"},
"encode": {
"update": {
"fill": {"value": "#4c78a8"},
"ariaRoleDescription": {"value": "bar"},
"description": {
"signal": "\"Year: \" + (timeFormat(datum[\"Year\"], '%b %d, %Y'))"
},
"xc": {"scale": "x", "field": "Year"},
"width": {"value": 5},
"y": {"value": 0},
"y2": {"field": {"group": "height"}}
}
}
}
],
"scales": [
{
"name": "x",
"type": "time",
"domain": {"data": "source_0", "field": "Year"},
"range": [0, {"signal": "width"}],
"padding": 5
}
],
"axes": [
{
"scale": "x",
"orient": "bottom",
"grid": true,
"tickCount": {"signal": "ceil(width/40)"},
"domain": false,
"labels": false,
"aria": false,
"maxExtent": 0,
"minExtent": 0,
"ticks": false,
"zindex": 0
},
{
"scale": "x",
"orient": "bottom",
"grid": false,
"title": "Year",
"labelFlush": true,
"labelOverlap": true,
"tickCount": {"signal": "ceil(width/40)"},
"zindex": 0
}
]
}
11 changes: 11 additions & 0 deletions examples/specs/bar_1d_temporal.vl.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"$schema": "https://vega.github.io/schema/vega-lite/v5.json",
"data": {"url": "data/cars.json"},
"mark": {"type": "bar", "orient": "vertical"},
"encoding": {
"x": {
"field": "Year",
"type": "temporal"
}
}
}
4 changes: 2 additions & 2 deletions src/channeldef.ts
Original file line number Diff line number Diff line change
Expand Up @@ -701,9 +701,9 @@ export function isContinuousFieldOrDatumDef<F extends Field>(
return (isTypedFieldDef(cd) && !isDiscrete(cd)) || isNumericDataDef(cd);
}

export function isQuantitativeFieldOrDatumDef<F extends Field>(cd: ChannelDef<F>) {
export function isUnbinnedQuantitativeFieldOrDatumDef<F extends Field>(cd: ChannelDef<F>) {
// TODO: make datum support DateTime object
return channelDefType(cd) === 'quantitative' || isNumericDataDef(cd);
return (isTypedFieldDef(cd) && cd.type === 'quantitative' && !cd.bin) || isNumericDataDef(cd);
}

export function isNumericDataDef<F extends Field>(cd: ChannelDef<F>): cd is DatumDef<F, number> {
Expand Down
40 changes: 16 additions & 24 deletions src/compile/mark/init.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {Orientation, SignalRef} from 'vega';
import {isBinned, isBinning} from '../../bin';
import {isContinuousFieldOrDatumDef, isFieldDef, isNumericDataDef, TypedFieldDef} from '../../channeldef';
import {isFieldDef, isNumericDataDef, isUnbinnedQuantitativeFieldOrDatumDef, isTypedFieldDef} from '../../channeldef';
import {Config} from '../../config';
import {Encoding, isAggregate} from '../../encoding';
import {replaceExprRef} from '../../expr';
Expand Down Expand Up @@ -178,39 +178,31 @@ function orient(mark: Mark, encoding: Encoding<string>, specifiedOrient: Orienta
// falls through
case LINE:
case TICK: {
// Tick is opposite to bar, line, area and never have ranged mark.
const xIsContinuous = isContinuousFieldOrDatumDef(x);
const yIsContinuous = isContinuousFieldOrDatumDef(y);
const xIsMeasure = isUnbinnedQuantitativeFieldOrDatumDef(x);
const yIsMeasure = isUnbinnedQuantitativeFieldOrDatumDef(y);

if (specifiedOrient) {
return specifiedOrient;
} else if (xIsContinuous && !yIsContinuous) {
} else if (xIsMeasure && !yIsMeasure) {
// Tick is opposite to bar, line, area
return mark !== 'tick' ? 'horizontal' : 'vertical';
} else if (!xIsContinuous && yIsContinuous) {
} else if (!xIsMeasure && yIsMeasure) {
// Tick is opposite to bar, line, area
return mark !== 'tick' ? 'vertical' : 'horizontal';
} else if (xIsContinuous && yIsContinuous) {
const xDef = x as TypedFieldDef<string>; // we can cast here since they are surely fieldDef
const yDef = y as TypedFieldDef<string>;

const xIsTemporal = xDef.type === TEMPORAL;
const yIsTemporal = yDef.type === TEMPORAL;
} else if (xIsMeasure && yIsMeasure) {
return 'vertical';
} else {
const xIsTemporal = isTypedFieldDef(x) && x.type === TEMPORAL;
const yIsTemporal = isTypedFieldDef(y) && y.type === TEMPORAL;

// temporal without timeUnit is considered continuous, but better serves as dimension
// x: T, y: N --> vertical tick
if (xIsTemporal && !yIsTemporal) {
return mark !== 'tick' ? 'vertical' : 'horizontal';
return 'vertical';
} else if (!xIsTemporal && yIsTemporal) {
return mark !== 'tick' ? 'horizontal' : 'vertical';
}

if (!xDef.aggregate && yDef.aggregate) {
return mark !== 'tick' ? 'vertical' : 'horizontal';
} else if (xDef.aggregate && !yDef.aggregate) {
return mark !== 'tick' ? 'horizontal' : 'vertical';
return 'horizontal';
}
return 'vertical';
} else {
return undefined;
}
return undefined;
}
}
return 'vertical';
Expand Down
9 changes: 9 additions & 0 deletions test/compile/mark/init.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,15 @@ describe('compile/mark/init', () => {
});
expect(model.markDef.orient).toBe('vertical');
});
it('should return correct default for T', () => {
const model = parseUnitModelWithScaleAndLayoutSize({
mark: 'bar',
encoding: {
x: {type: 'temporal', field: 'bar'}
}
});
expect(model.markDef.orient).toBe('vertical');
});

it('should return correct default for empty plot', () => {
const model = parseUnitModelWithScaleAndLayoutSize({
Expand Down

0 comments on commit ea4aa64

Please sign in to comment.