diff --git a/src/channeldef.ts b/src/channeldef.ts index 8e4b45ffae..15f61dbfef 100644 --- a/src/channeldef.ts +++ b/src/channeldef.ts @@ -58,7 +58,7 @@ import * as log from './log'; import {LogicalComposition} from './logical'; import {isRectBasedMark, Mark, MarkDef} from './mark'; import {Predicate} from './predicate'; -import {Scale, SCALE_CATEGORY_INDEX} from './scale'; +import {isContinuousToDiscrete, Scale, SCALE_CATEGORY_INDEX} from './scale'; import {isSortByChannel, Sort, SortOrder} from './sort'; import {isFacetFieldDef} from './spec/facet'; import {StackOffset, StackProperties} from './stack'; @@ -657,7 +657,7 @@ export function isContinuousFieldOrDatumDef( cd: ChannelDef ): cd is TypedFieldDef | DatumDef { // TODO: make datum support DateTime object - return (isTypedFieldDef(cd) && isContinuous(cd)) || isNumericDataDef(cd); + return (isTypedFieldDef(cd) && !isDiscrete(cd)) || isNumericDataDef(cd); } export function isQuantitativeFieldOrDatumDef(cd: ChannelDef) { @@ -815,8 +815,8 @@ export function isDiscrete(def: TypedFieldDef | DatumDef) { throw new Error(log.message.invalidFieldType(def.type)); } -export function isContinuous(fieldDef: TypedFieldDef) { - return !isDiscrete(fieldDef); +export function isDiscretizing(def: TypedFieldDef | DatumDef) { + return isScaleFieldDef(def) && isContinuousToDiscrete(def.scale?.type); } export function isCount(fieldDef: FieldDefBase) { @@ -1204,10 +1204,10 @@ export function channelCompatibility( case ROW: case COLUMN: case FACET: - if (isContinuous(fieldDef)) { + if (!isDiscrete(fieldDef)) { return { compatible: false, - warning: log.message.facetChannelShouldBeDiscrete(channel) + warning: log.message.channelShouldBeDiscrete(channel) }; } return COMPATIBLE; @@ -1258,20 +1258,12 @@ export function channelCompatibility( } return COMPATIBLE; - case STROKEDASH: - if (!['ordinal', 'nominal'].includes(fieldDef.type)) { - return { - compatible: false, - warning: 'StrokeDash channel should be used with only discrete data.' - }; - } - return COMPATIBLE; - case SHAPE: - if (!['ordinal', 'nominal', 'geojson'].includes(fieldDef.type)) { + case STROKEDASH: + if (!isDiscrete(fieldDef) && !isDiscretizing(fieldDef)) { return { compatible: false, - warning: 'Shape channel should be used with only either discrete or geojson data.' + warning: log.message.channelShouldBeDiscreteOrDiscretizing(channel) }; } return COMPATIBLE; diff --git a/src/log/message.ts b/src/log/message.ts index 65e7e881a3..bd79c7ced3 100644 --- a/src/log/message.ts +++ b/src/log/message.ts @@ -188,10 +188,14 @@ export function invalidEncodingChannel(channel: ExtendedChannel) { return `${channel}-encoding is dropped as ${channel} is not a valid encoding channel.`; } -export function facetChannelShouldBeDiscrete(channel: FacetChannel) { +export function channelShouldBeDiscrete(channel: ExtendedChannel) { return `${channel} encoding should be discrete (ordinal / nominal / binned).`; } +export function channelShouldBeDiscreteOrDiscretizing(channel: ExtendedChannel) { + return `${channel} encoding should be discrete (ordinal / nominal / binned) or use a discretizing scale (e.g. threshold).`; +} + export function facetChannelDropped(channels: FacetChannel[]) { return `Facet encoding dropped as ${channels.join(' and ')} ${channels.length > 1 ? 'are' : 'is'} also specified.`; } diff --git a/src/scale.ts b/src/scale.ts index 3a8e80d6ef..00f20c55ad 100644 --- a/src/scale.ts +++ b/src/scale.ts @@ -70,7 +70,7 @@ export const SCALE_CATEGORY_INDEX: Record { } }); expect(model.facet).toEqual({row: {field: 'a', type: 'quantitative'}}); - expect(localLogger.warns[0]).toEqual(log.message.facetChannelShouldBeDiscrete(ROW)); + expect(localLogger.warns[0]).toEqual(log.message.channelShouldBeDiscrete(ROW)); }) ); diff --git a/test/scale.test.ts b/test/scale.test.ts index ac4db3c47e..7f167e0493 100644 --- a/test/scale.test.ts +++ b/test/scale.test.ts @@ -1,6 +1,12 @@ import {ScaleChannel, SCALE_CHANNELS} from '../src/channel'; import * as scale from '../src/scale'; -import {channelSupportScaleType, CONTINUOUS_TO_CONTINUOUS_SCALES, ScaleType, SCALE_TYPES} from '../src/scale'; +import { + channelSupportScaleType, + CONTINUOUS_TO_CONTINUOUS_SCALES, + CONTINUOUS_TO_DISCRETE_SCALES, + ScaleType, + SCALE_TYPES +} from '../src/scale'; import {some} from '../src/util'; import {without} from './util'; @@ -51,14 +57,28 @@ describe('scale', () => { } }); - it('shape should support only ordinal', () => { - expect(channelSupportScaleType('shape', 'ordinal')).toBeTruthy(); - const nonOrdinal = without(SCALE_TYPES, ['ordinal']); - for (const scaleType of nonOrdinal) { + it('shape should support only ordinal and discretizing scales', () => { + const supportedScales = [ScaleType.ORDINAL, ...CONTINUOUS_TO_DISCRETE_SCALES] as const; + for (const scaleType of supportedScales) { + expect(channelSupportScaleType('shape', scaleType)).toBeTruthy(); + } + const unsupported = without(SCALE_TYPES, supportedScales); + for (const scaleType of unsupported) { expect(!channelSupportScaleType('shape', scaleType)).toBeTruthy(); } }); + it('strokeDash should support only ordinal and discretizing scales', () => { + const supportedScales = [ScaleType.ORDINAL, ...CONTINUOUS_TO_DISCRETE_SCALES] as const; + for (const scaleType of supportedScales) { + expect(channelSupportScaleType('strokeDash', scaleType)).toBeTruthy(); + } + const unsupported = without(SCALE_TYPES, supportedScales); + for (const scaleType of unsupported) { + expect(!channelSupportScaleType('strokeDash', scaleType)).toBeTruthy(); + } + }); + it('color should support all scale types except band', () => { for (const scaleType of SCALE_TYPES) { expect(channelSupportScaleType('color', scaleType)).toEqual(scaleType !== 'band');