Skip to content

Commit

Permalink
fix: Allow non-linear scale for stacked bars (#9315)
Browse files Browse the repository at this point in the history
Clone of #9311 by @richardliu-db to fix CI issues. See that PR for
details.

---------

Signed-off-by: Lukas Hermann <lukas.hermann@databricks.com>
Co-authored-by: GitHub Actions Bot <vega-actions-bot@users.noreply.github.com>
  • Loading branch information
lsh and GitHub Actions Bot committed Apr 17, 2024
1 parent 8b3b30b commit 716fe73
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 32 deletions.
14 changes: 11 additions & 3 deletions examples/compiled/bar_q_qpow.vg.json
Expand Up @@ -18,6 +18,14 @@
"name": "data_0",
"source": "source_0",
"transform": [
{
"type": "stack",
"groupby": ["a"],
"field": "b",
"sort": {"field": [], "order": []},
"as": ["b_start", "b_end"],
"offset": "zero"
},
{
"type": "filter",
"expr": "isValid(datum[\"a\"]) && isFinite(+datum[\"a\"]) && isValid(datum[\"b\"]) && isFinite(+datum[\"b\"])"
Expand All @@ -40,8 +48,8 @@
},
"xc": {"scale": "x", "field": "a"},
"width": {"value": 5},
"y": {"scale": "y", "field": "b"},
"y2": {"scale": "y", "value": 0}
"y": {"scale": "y", "field": "b_end"},
"y2": {"scale": "y", "field": "b_start"}
}
}
}
Expand All @@ -59,7 +67,7 @@
{
"name": "y",
"type": "pow",
"domain": {"data": "data_0", "field": "b"},
"domain": {"data": "data_0", "fields": ["b_start", "b_end"]},
"range": [{"signal": "height"}, 0],
"nice": true,
"zero": true
Expand Down
4 changes: 2 additions & 2 deletions src/log/message.ts
Expand Up @@ -344,8 +344,8 @@ export function cannotStackRangedMark(channel: Channel) {
return `Cannot stack "${channel}" if there is already "${channel}2".`;
}

export function cannotStackNonLinearScale(scaleType: ScaleType) {
return `Cannot stack non-linear scale (${scaleType}).`;
export function stackNonLinearScale(scaleType: ScaleType) {
return `Stack is applied to a non-linear scale (${scaleType}).`;
}

export function stackNonSummativeAggregate(aggregate: Aggregate | string) {
Expand Down
3 changes: 1 addition & 2 deletions src/stack.ts
Expand Up @@ -245,9 +245,8 @@ export function stack(m: Mark | MarkDef, encoding: Encoding<string>): StackPrope
// warn when stacking non-linear
if (stackedFieldDef?.scale?.type && stackedFieldDef?.scale?.type !== ScaleType.LINEAR) {
if (stackedFieldDef?.stack) {
log.warn(log.message.cannotStackNonLinearScale(stackedFieldDef.scale.type));
log.warn(log.message.stackNonLinearScale(stackedFieldDef.scale.type));
}
return null;
}

// Check if it is a ranged mark
Expand Down
34 changes: 9 additions & 25 deletions test/stack.test.ts
Expand Up @@ -256,9 +256,9 @@ describe('stack', () => {
});

it(
'should always warn if the aggregated axis has non-linear scale',
'should always return stack and only warn for non-undefined stack if the aggregated axis has non-linear scale',
log.wrap(localLogger => {
for (const s of ['center', 'zero', 'normalize'] as const) {
for (const s of [undefined, 'center', 'zero', 'normalize'] as const) {
for (const scaleType of [ScaleType.LOG, ScaleType.POW, ScaleType.SQRT]) {
const marks = s === undefined ? STACK_BY_DEFAULT_NON_POLAR_MARKS : STACKABLE_NON_POLAR_MARKS;
for (const mark of marks) {
Expand All @@ -271,36 +271,20 @@ describe('stack', () => {
color: {field: 'site', type: 'nominal'}
}
};
expect(stack(spec.mark, spec.encoding)).toBeNull();

expect(stack(spec.mark, spec.encoding)).toBeTruthy();
const warns = localLogger.warns;
expect(warns[warns.length - 1]).toEqual(log.message.cannotStackNonLinearScale(scaleType));

if (s !== undefined) {
expect(warns[warns.length - 1]).toEqual(log.message.stackNonLinearScale(scaleType));
} else {
expect(warns).toHaveLength(0);
}
}
}
}
})
);

it('returns null if the aggregated axis has non-linear scale', () => {
for (const stacked of [undefined, 'center', 'zero', 'normalize'] as const) {
for (const scaleType of [ScaleType.LOG, ScaleType.POW, ScaleType.SQRT]) {
const marks = stacked === undefined ? STACK_BY_DEFAULT_NON_POLAR_MARKS : STACKABLE_NON_POLAR_MARKS;
for (const mark of marks) {
const spec: TopLevel<NormalizedUnitSpec> = {
data: {url: 'data/barley.json'},
mark,
encoding: {
x: {field: 'a', type: 'quantitative', aggregate: 'sum', scale: {type: scaleType}},
y: {field: 'variety', type: 'nominal'},
color: {field: 'site', type: 'nominal'}
}
};
expect(stack(spec.mark, spec.encoding)).toBeNull();
}
}
}
});

it(
'should throw warning if the aggregated axis has a non-summative aggregate',
log.wrap(localLogger => {
Expand Down

0 comments on commit 716fe73

Please sign in to comment.