fix: support discretizing scales for shape (#7170)

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<F extends Field>(
   cd: ChannelDef<F>
 ): cd is TypedFieldDef<F> | DatumDef<F, number> {
   // TODO: make datum support DateTime object
-  return (isTypedFieldDef(cd) && isContinuous(cd)) || isNumericDataDef(cd);
+  return (isTypedFieldDef(cd) && !isDiscrete(cd)) || isNumericDataDef(cd);
 }
 
 export function isQuantitativeFieldOrDatumDef<F extends Field>(cd: ChannelDef<F>) {
@@ -815,8 +815,8 @@ export function isDiscrete(def: TypedFieldDef<Field> | DatumDef<any, any>) {
   throw new Error(log.message.invalidFieldType(def.type));
 }
 
-export function isContinuous(fieldDef: TypedFieldDef<Field>) {
-  return !isDiscrete(fieldDef);
+export function isDiscretizing(def: TypedFieldDef<Field> | DatumDef<any, any>) {
+  return isScaleFieldDef(def) && isContinuousToDiscrete(def.scale?.type);
 }
 
 export function isCount(fieldDef: FieldDefBase<Field>) {
@@ -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;

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.`;
 }

src/scale.ts

@@ -70,7 +70,7 @@ export const SCALE_CATEGORY_INDEX: Record<ScaleType, ScaleType | 'numeric' | 'or
   threshold: 'discretizing'
 };
 
-export const SCALE_TYPES = keys(SCALE_CATEGORY_INDEX) as ScaleType[];
+export const SCALE_TYPES: ScaleType[] = keys(SCALE_CATEGORY_INDEX);
 
 /**
  * Whether the two given scale types can be merged together.
@@ -839,8 +839,7 @@ export function channelSupportScaleType(channel: Channel, scaleType: ScaleType):
     case CHANNEL.STROKE:
       return scaleType !== 'band'; // band does not make sense with color
     case CHANNEL.STROKEDASH:
-      return scaleType === 'ordinal' || isContinuousToDiscrete(scaleType);
     case CHANNEL.SHAPE:
-      return scaleType === 'ordinal'; // shape = lookup only
+      return scaleType === 'ordinal' || isContinuousToDiscrete(scaleType);
   }
 }

test/compile/facet.test.ts

@@ -40,7 +40,7 @@ describe('FacetModel', () => {
           }
         });
         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));
       })
     );
 

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<ScaleType>(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');