From eb13dcf0751929690d3019e5d6a25438a73ebdfc Mon Sep 17 00:00:00 2001 From: Dominik Moritz Date: Sat, 19 Feb 2022 14:36:51 -0500 Subject: [PATCH] fix: do not stack non linear scales --- src/stack.ts | 17 +++-------------- test/stack.test.ts | 36 ++++++++++++++++++------------------ 2 files changed, 21 insertions(+), 32 deletions(-) diff --git a/src/stack.ts b/src/stack.ts index c495c6d9ff9..89035980b20 100644 --- a/src/stack.ts +++ b/src/stack.ts @@ -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, - opt: { - disallowNonLinearStack?: boolean; // This option is for CompassQL - } = {} -): StackProperties { +export function stack(m: Mark | MarkDef, encoding: Encoding): StackProperties { const mark = isMarkDef(m) ? m.type : m; // Should have stackable mark if (!STACKABLE_MARKS.has(mark)) { @@ -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 diff --git a/test/stack.test.ts b/test/stack.test.ts index 4afa9d2a8c7..e66556691f8 100644 --- a/test/stack.test.ts +++ b/test/stack.test.ts @@ -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); } }); @@ -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(); } }); @@ -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(); } }); @@ -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); } }); @@ -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); } }); @@ -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); @@ -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) { @@ -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)); @@ -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; @@ -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(); } } } @@ -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)); @@ -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]); } @@ -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([]); } @@ -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]); } @@ -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([]); } @@ -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'); }); @@ -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'); }); }); @@ -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'); } } });