Skip to content

Commit

Permalink
fix: do not stack non linear scales
Browse files Browse the repository at this point in the history
  • Loading branch information
@domoritz
domoritz committed Feb 23, 2022
1 parent ae13aff commit cf7b1d6
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 32 deletions.
17 changes: 3 additions & 14 deletions src/stack.ts
View file
Expand Up @@ -137,15 +137,7 @@ function getDimensionChannel(channel: 'x' | 'y' | 'theta' | 'radius') {
}
}

// Note: CompassQL uses this method and only pass in required properties of each argument object.
// If required properties change, make sure to update CompassQL.
export function stack(
m: Mark | MarkDef,
encoding: Encoding<string>,
opt: {
disallowNonLinearStack?: boolean; // This option is for CompassQL
} = {}
): StackProperties {
export function stack(m: Mark | MarkDef, encoding: Encoding<string>): StackProperties {
const mark = isMarkDef(m) ? m.type : m;
// Should have stackable mark
if (!STACKABLE_MARKS.has(mark)) {
Expand Down Expand Up @@ -241,11 +233,8 @@ export function stack(

// warn when stacking non-linear
if (stackedFieldDef?.scale?.type && stackedFieldDef?.scale?.type !== ScaleType.LINEAR) {
if (opt.disallowNonLinearStack) {
return null;
} else {
log.warn(log.message.cannotStackNonLinearScale(stackedFieldDef.scale.type));
}
log.warn(log.message.cannotStackNonLinearScale(stackedFieldDef.scale.type));
return null;
}

// Check if it is a ranged mark
Expand Down
36 changes: 18 additions & 18 deletions test/stack.test.ts
View file
Expand Up @@ -37,7 +37,7 @@ describe('stack', () => {
color: {field: 'site', type: 'nominal'}
}
};
const stackProps = stack(spec.mark, spec.encoding, undefined);
const stackProps = stack(spec.mark, spec.encoding);
expect(stackProps.fieldChannel).toBe(X);
}
});
Expand All @@ -51,7 +51,7 @@ describe('stack', () => {
x: {field: 'yield', type: 'quantitative', bin: true}
}
};
const stackProps = stack(spec.mark, spec.encoding, undefined);
const stackProps = stack(spec.mark, spec.encoding);
expect(stackProps).toBeNull();
}
});
Expand All @@ -67,7 +67,7 @@ describe('stack', () => {
color: {field: 'site', type: 'nominal'}
}
};
const stackProps = stack(spec.mark, spec.encoding, undefined);
const stackProps = stack(spec.mark, spec.encoding);
expect(stackProps).toBeNull();
}
});
Expand All @@ -82,7 +82,7 @@ describe('stack', () => {
y: {field: 'variety', type: 'nominal'}
}
};
const stackProps = stack(spec.mark, spec.encoding, undefined);
const stackProps = stack(spec.mark, spec.encoding);
expect(stackProps.fieldChannel).toBe(X);
}
});
Expand All @@ -98,7 +98,7 @@ describe('stack', () => {
color: {field: 'site', type: 'nominal'}
}
};
const stackProps = stack(spec.mark, spec.encoding, undefined);
const stackProps = stack(spec.mark, spec.encoding);
expect(stackProps.fieldChannel).toBe(X);
}
});
Expand Down Expand Up @@ -137,7 +137,7 @@ describe('stack', () => {
}
};

const _stack = stack(spec.mark, spec.encoding, undefined);
const _stack = stack(spec.mark, spec.encoding);
expect(_stack).toBeTruthy();

expect(_stack.stackBy[0].channel).toEqual(DETAIL);
Expand All @@ -157,7 +157,7 @@ describe('stack', () => {
}
};

const _stack = stack(spec.mark, spec.encoding, undefined);
const _stack = stack(spec.mark, spec.encoding);
expect(_stack).toBeTruthy();

for (const stackBy of _stack.stackBy) {
Expand Down Expand Up @@ -247,7 +247,7 @@ describe('stack', () => {
color: {field: 'site', type: 'nominal'}
}
};
expect(stack(spec.mark, spec.encoding)).not.toBeNull();
expect(stack(spec.mark, spec.encoding)).toBeNull();

const warns = localLogger.warns;
expect(warns[warns.length - 1]).toEqual(log.message.cannotStackNonLinearScale(scaleType));
Expand All @@ -257,7 +257,7 @@ describe('stack', () => {
})
);

it('returns null if the aggregated axis has non-linear scale and disallowNonLinearStack = true', () => {
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;
Expand All @@ -271,7 +271,7 @@ describe('stack', () => {
color: {field: 'site', type: 'nominal'}
}
};
expect(stack(spec.mark, spec.encoding, {disallowNonLinearStack: true})).toBeNull();
expect(stack(spec.mark, spec.encoding)).toBeNull();
}
}
}
Expand Down Expand Up @@ -299,7 +299,7 @@ describe('stack', () => {
}
};

stack(spec.mark, spec.encoding, undefined);
stack(spec.mark, spec.encoding);

const warns = localLogger.warns;
expect(warns[warns.length - 1]).toEqual(log.message.stackNonSummativeAggregate(aggregate));
Expand All @@ -321,7 +321,7 @@ describe('stack', () => {
color: {field: 'site', type: 'nominal'}
}
};
const _stack = stack(spec.mark, spec.encoding, undefined);
const _stack = stack(spec.mark, spec.encoding);
expect(_stack.fieldChannel).toBe(X);
expect(_stack.groupbyChannels).toEqual([Y]);
}
Expand All @@ -337,7 +337,7 @@ describe('stack', () => {
color: {field: 'site', type: 'nominal'}
}
};
const _stack = stack(spec.mark, spec.encoding, undefined);
const _stack = stack(spec.mark, spec.encoding);
expect(_stack.fieldChannel).toBe(X);
expect(_stack.groupbyChannels).toEqual([]);
}
Expand All @@ -354,7 +354,7 @@ describe('stack', () => {
color: {field: 'site', type: 'nominal'}
}
};
const _stack = stack(spec.mark, spec.encoding, undefined);
const _stack = stack(spec.mark, spec.encoding);
expect(_stack.fieldChannel).toBe(Y);
expect(_stack.groupbyChannels).toEqual([X]);
}
Expand All @@ -370,7 +370,7 @@ describe('stack', () => {
color: {field: 'site', type: 'nominal'}
}
};
const _stack = stack(spec.mark, spec.encoding, undefined);
const _stack = stack(spec.mark, spec.encoding);
expect(_stack.fieldChannel).toBe(Y);
expect(_stack.groupbyChannels).toEqual([]);
}
Expand All @@ -385,7 +385,7 @@ describe('stack', () => {
color: {field: 'id', type: 'nominal'}
}
};
const _stack = stack(spec.mark, spec.encoding, undefined);
const _stack = stack(spec.mark, spec.encoding);
expect(_stack.fieldChannel).toBe('theta');
expect(_stack.stackBy[0].channel).toBe('color');
});
Expand All @@ -403,7 +403,7 @@ describe('stack', () => {
}
}
};
const _stack = stack(spec.mark, spec.encoding, undefined);
const _stack = stack(spec.mark, spec.encoding);
expect(_stack.fieldChannel).toBe('theta');
});
});
Expand All @@ -421,7 +421,7 @@ describe('stack', () => {
color: {field: 'site', type: 'nominal'}
}
};
expect(stack(spec.mark, spec.encoding, undefined).offset).toBe('zero');
expect(stack(spec.mark, spec.encoding).offset).toBe('zero');
}
}
});
Expand Down

0 comments on commit cf7b1d6

Please sign in to comment.