Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable checking on scale type if is supported by data type #2080

Merged
merged 1 commit into from
Apr 5, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 31 additions & 9 deletions src/compile/scale/type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@ import {ScaleConfig, ScaleType} from '../../scale';
import {isDiscreteByDefault} from '../../timeunit';

import {FieldDef} from '../../fielddef';
import {hasDiscreteDomain} from '../../scale';
import {Type} from '../../type';
import * as util from '../../util';
import {contains} from '../../util';

export type RangeType = 'continuous' | 'discrete' | 'flexible' | undefined;

Expand All @@ -26,20 +29,19 @@ export default function type(
return null;
}
if (specifiedType !== undefined) {
// for binned fields we don't allow overriding the default scale
if (fieldDef.bin) {
// TODO: generalize this as a method in fieldDef that determines scale type support for a fieldDef (looking at functions and type)
log.warn(log.message.cannotOverrideBinScaleType(channel, defaultScaleType));
// Check if explicitly specified scale type is supported by the channel
if (!supportScaleType(channel, specifiedType)) {
log.warn(log.message.scaleTypeNotWorkWithChannel(channel, specifiedType, defaultScaleType));
return defaultScaleType;
}

// Check if explicitly specified scale type is supported by the channel
if (supportScaleType(channel, specifiedType)) {
return specifiedType;
} else {
log.warn(log.message.scaleTypeNotWorkWithChannel(channel, specifiedType, defaultScaleType));
// Check if explicitly specified scale type is supported by the data type
if (!fieldDefMatchScaleType(specifiedType, fieldDef)) {
log.warn(log.message.scaleTypeNotWorkWithFieldDef(specifiedType, defaultScaleType));
return defaultScaleType;
}

return specifiedType;
}

return defaultScaleType;
Expand Down Expand Up @@ -146,3 +148,23 @@ function haveRangeStep(hasTopLevelSize: boolean, specifiedRangeStep: number, sca
}
return !!scaleConfig.rangeStep;
}

export function fieldDefMatchScaleType(specifiedType: ScaleType, fieldDef: FieldDef):boolean {
const type: Type = fieldDef.type;
if (contains([Type.ORDINAL, Type.NOMINAL], type)) {
return specifiedType === undefined || hasDiscreteDomain(specifiedType);
} else if (type === Type.TEMPORAL) {
if (!fieldDef.timeUnit) {
return contains([ScaleType.TIME, ScaleType.UTC, undefined], specifiedType);
} else {
return contains([ScaleType.TIME, ScaleType.UTC, undefined], specifiedType) || hasDiscreteDomain(specifiedType);
}
} else if (type === Type.QUANTITATIVE) {
if (fieldDef.bin) {
return specifiedType === ScaleType.BIN_LINEAR || specifiedType === ScaleType.BIN_ORDINAL;
}
return contains([ScaleType.LOG, ScaleType.POW, ScaleType.SQRT, ScaleType.QUANTILE, ScaleType.QUANTIZE, ScaleType.LINEAR, undefined], specifiedType);
}

return true;
}
8 changes: 4 additions & 4 deletions src/log.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,14 +173,14 @@ export namespace message {
channel === 'x' ? 'width' : 'height'} is provided.`;
}

export function cannotOverrideBinScaleType(channel: Channel, defaultScaleType: ScaleType) {
return `Cannot override scale type for binned channel ${channel}. We are using ${defaultScaleType} scale instead.`;
}

export function scaleTypeNotWorkWithChannel(channel: Channel, scaleType: ScaleType, defaultScaleType: ScaleType) {
return `Channel ${channel} does not work with ${scaleType} scale. We are using ${defaultScaleType} scale instead.`;
}

export function scaleTypeNotWorkWithFieldDef(scaleType: ScaleType, defaultScaleType: ScaleType) {
return `FieldDef does not work with ${scaleType} scale. We are using ${defaultScaleType} scale instead.`;
}

export function scalePropertyNotWorkWithScaleType(scaleType: ScaleType, propName: string, channel: Channel) {
return `${channel}-scale's "${propName}" is dropped as it does not work with ${scaleType} scale.`;
}
Expand Down
58 changes: 57 additions & 1 deletion test/compile/scale/type.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ describe('compile/scale', () => {
scaleType('point', 'color', {type: 'quantitative', bin: 'true'}, 'point', undefined, undefined, defaultConfig),
ScaleType.BIN_ORDINAL
);
assert.equal(localLogger.warns[0], log.message.cannotOverrideBinScaleType('color', 'bin-ordinal'));
assert.equal(localLogger.warns[0], log.message.scaleTypeNotWorkWithFieldDef(ScaleType.POINT, ScaleType.BIN_ORDINAL));
});
});

Expand Down Expand Up @@ -273,5 +273,61 @@ describe('compile/scale', () => {
);
});
});

describe('dataTypeMatchScaleType()', () => {
it('should return specified value if datatype is ordinal or nominal and specified scale type is the ordinal or nominal', () => {
assert.equal(
scaleType(ScaleType.ORDINAL, 'shape', {type: 'ordinal'}, 'point', undefined, undefined, defaultConfig),
ScaleType.ORDINAL
);
});

it('should return default scale type if data type is temporal but specified scale type is not time or utc', () => {
assert.equal(
scaleType(ScaleType.LINEAR, 'x', {type: 'temporal', timeUnit: 'year'}, 'point', undefined, undefined, defaultConfig),
ScaleType.TIME
);

assert.equal(
scaleType(ScaleType.LINEAR, 'color', {type: 'temporal', timeUnit: 'year'}, 'point', undefined, undefined, defaultConfig),
ScaleType.SEQUENTIAL
);
});

it('should return specified discrete scale type if data type is temporal but specified scale type is time or utc', () => {
assert.equal(
scaleType(ScaleType.POINT, 'x', {type: 'temporal', timeUnit: 'year'}, 'point', undefined, undefined, defaultConfig),
ScaleType.POINT
);
});

it('should return default scale type if data type is temporal but specified scale type is time or utc or any discrete type', () => {
assert.equal(
scaleType(ScaleType.LINEAR, 'x', {type: 'temporal', timeUnit: 'year'}, 'point', undefined, undefined, defaultConfig),
ScaleType.TIME
);
});

it('should return default scale type if data type is quantative but scale type do not support quantative', () => {
assert.equal(
scaleType(ScaleType.TIME, 'color', {type: 'quantitative'}, 'point', undefined, undefined, defaultConfig),
ScaleType.SEQUENTIAL
);
});

it('should return default scale type if data type is quantative and scale type supports quantative', () => {
assert.equal(
scaleType(ScaleType.TIME, 'x', {type: 'quantitative'}, 'point', undefined, undefined, defaultConfig),
ScaleType.LINEAR
);
});

it('should return default scale type if data type is quantative and scale type supports quantative', () => {
assert.equal(
scaleType(ScaleType.TIME, 'x', {type: 'temporal'}, 'point', undefined, undefined, defaultConfig),
ScaleType.TIME
);
});
});
});
});