Skip to content

Commit

Permalink
fix(specs): limit xScaleType to linear, time and ordinal (#127)
Browse files Browse the repository at this point in the history 
We no longer accept a log or sqrt scale type for the xScaleType props in every
bar/area/line components

close #122
  • Loading branch information
@markov00
markov00 authored Mar 29, 2019
1 parent 32b4263 commit 59c3b70
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 142 deletions.
132 changes: 13 additions & 119 deletions src/lib/series/domains/x_domain.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,57 +71,6 @@ describe('X Domain', () => {
isBandScale: false,
});
});
test('Should return correct scale type with multi line, area (log)', () => {
const seriesSpecs: Array<Pick<BasicSeriesSpec, 'seriesType' | 'xScaleType'>> = [
{
seriesType: 'line',
xScaleType: ScaleType.Log,
},
{
seriesType: 'area',
xScaleType: ScaleType.Log,
},
];
const mainXScale = convertXScaleTypes(seriesSpecs);
expect(mainXScale).toEqual({
scaleType: ScaleType.Log,
isBandScale: false,
});
});
test('Should return correct scale type with multi line, area with different scale types (time, log)', () => {
const seriesSpecs: Array<Pick<BasicSeriesSpec, 'seriesType' | 'xScaleType'>> = [
{
seriesType: 'line',
xScaleType: ScaleType.Time,
},
{
seriesType: 'area',
xScaleType: ScaleType.Log,
},
];
const mainXScale = convertXScaleTypes(seriesSpecs);
expect(mainXScale).toEqual({
scaleType: ScaleType.Linear,
isBandScale: false,
});
});
test('Should return correct scale type with multi line, area with different scale types (ordinal, log)', () => {
const seriesSpecs: Array<Pick<BasicSeriesSpec, 'seriesType' | 'xScaleType'>> = [
{
seriesType: 'line',
xScaleType: ScaleType.Ordinal,
},
{
seriesType: 'area',
xScaleType: ScaleType.Log,
},
];
const mainXScale = convertXScaleTypes(seriesSpecs);
expect(mainXScale).toEqual({
scaleType: ScaleType.Ordinal,
isBandScale: false,
});
});
test('Should return correct scale type with multi line with different scale types (linear, ordinal)', () => {
const seriesSpecs: Array<Pick<BasicSeriesSpec, 'seriesType' | 'xScaleType'>> = [
{
Expand All @@ -139,15 +88,15 @@ describe('X Domain', () => {
isBandScale: false,
});
});
test('Should return correct scale type with multi bar, area with different scale types (ordinal, log)', () => {
test('Should return correct scale type with multi bar, area with different scale types (linear, ordinal)', () => {
const seriesSpecs: Array<Pick<BasicSeriesSpec, 'seriesType' | 'xScaleType'>> = [
{
seriesType: 'bar',
xScaleType: ScaleType.Ordinal,
xScaleType: ScaleType.Linear,
},
{
seriesType: 'area',
xScaleType: ScaleType.Log,
xScaleType: ScaleType.Ordinal,
},
];
const mainXScale = convertXScaleTypes(seriesSpecs);
Expand All @@ -156,37 +105,20 @@ describe('X Domain', () => {
isBandScale: true,
});
});
test('Should return correct scale type with multi bar, area with different scale types (time, log)', () => {
const seriesSpecs: Array<Pick<BasicSeriesSpec, 'seriesType' | 'xScaleType'>> = [
{
seriesType: 'bar',
xScaleType: ScaleType.Time,
},
{
seriesType: 'area',
xScaleType: ScaleType.Log,
},
];
const mainXScale = convertXScaleTypes(seriesSpecs);
expect(mainXScale).toEqual({
scaleType: ScaleType.Linear,
isBandScale: true,
});
});
test('Should return correct scale type with multi bar, area with different scale types (linear, ordinal)', () => {
test('Should return correct scale type with multi bar, area with same scale types (linear, linear)', () => {
const seriesSpecs: Array<Pick<BasicSeriesSpec, 'seriesType' | 'xScaleType'>> = [
{
seriesType: 'bar',
xScaleType: ScaleType.Linear,
},
{
seriesType: 'area',
xScaleType: ScaleType.Ordinal,
xScaleType: ScaleType.Time,
},
];
const mainXScale = convertXScaleTypes(seriesSpecs);
expect(mainXScale).toEqual({
scaleType: ScaleType.Ordinal,
scaleType: ScaleType.Linear,
isBandScale: true,
});
});
Expand Down Expand Up @@ -519,48 +451,6 @@ describe('X Domain', () => {
);
expect(mergedDomain.domain).toEqual([0, 1, 2, 5, 7]);
});
test('Should merge multi lines log/linear series correctly', () => {
const ds1: BasicSeriesSpec = {
id: getSpecId('ds1'),
groupId: getGroupId('g1'),
seriesType: 'line',
xAccessor: 'x',
yAccessors: ['y'],
xScaleType: ScaleType.Log,
yScaleType: ScaleType.Linear,
yScaleToDataExtent: false,
data: [{ x: 0, y: 0 }, { x: 1, y: 0 }, { x: 2, y: 0 }, { x: 5, y: 0 }],
};
const ds2: BasicSeriesSpec = {
id: getSpecId('ds2'),
groupId: getGroupId('g2'),
seriesType: 'line',
xAccessor: 'x',
yAccessors: ['y'],
xScaleType: ScaleType.Linear,
yScaleType: ScaleType.Linear,
yScaleToDataExtent: false,
data: [{ x: 0, y: 0 }, { x: 7, y: 0 }],
};
const specDataSeries = new Map<SpecId, BasicSeriesSpec>();
specDataSeries.set(ds1.id, ds1);
specDataSeries.set(ds2.id, ds2);
const { xValues } = getSplittedSeries(specDataSeries);
const mergedDomain = mergeXDomain(
[
{
seriesType: 'line',
xScaleType: ScaleType.Log,
},
{
seriesType: 'line',
xScaleType: ScaleType.Linear,
},
],
xValues,
);
expect(mergedDomain.domain).toEqual([0, 7]);
});

test('Should merge X multi high volume of data', () => {
const maxValues = 10000;
Expand All @@ -570,7 +460,7 @@ describe('X Domain', () => {
seriesType: 'line',
xAccessor: 'x',
yAccessors: ['y'],
xScaleType: ScaleType.Log,
xScaleType: ScaleType.Linear,
yScaleType: ScaleType.Linear,
yScaleToDataExtent: false,
data: new Array(maxValues).fill(0).map((d, i) => ({ x: i, y: i })),
Expand Down Expand Up @@ -673,7 +563,9 @@ describe('X Domain', () => {
const attemptToMerge = () => {
mergeXDomain(specs, xValues, invalidXDomain);
};
expect(attemptToMerge).toThrowError('custom xDomain is invalid, custom min is greater than computed max');
expect(attemptToMerge).toThrowError(
'custom xDomain is invalid, custom min is greater than computed max',
);
});

test('should account for custom domain when merging a linear domain: upper bounded domain', () => {
Expand All @@ -690,7 +582,9 @@ describe('X Domain', () => {
const attemptToMerge = () => {
mergeXDomain(specs, xValues, invalidXDomain);
};
expect(attemptToMerge).toThrowError('custom xDomain is invalid, computed min is greater than custom max');
expect(attemptToMerge).toThrowError(
'custom xDomain is invalid, computed min is greater than custom max',
);
});

test('should account for custom domain when merging an ordinal domain', () => {
Expand Down
26 changes: 13 additions & 13 deletions src/lib/series/domains/x_domain.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ export function mergeXDomain(
if (Array.isArray(xDomain)) {
seriesXComputedDomains = xDomain;
} else {
throw new Error('xDomain for ordinal scale should be an array of values, not a DomainRange object');
throw new Error(
'xDomain for ordinal scale should be an array of values, not a DomainRange object',
);
}
}
} else {
Expand Down Expand Up @@ -65,7 +67,9 @@ export function mergeXDomain(
seriesXComputedDomains = [computedDomainMin, xDomain.max];
}
} else {
throw new Error('xDomain for continuous scale should be a DomainRange object, not an array');
throw new Error(
'xDomain for continuous scale should be a DomainRange object, not an array',
);
}
}
minInterval = findMinInterval(values);
Expand All @@ -85,27 +89,23 @@ export function mergeXDomain(
* Default to 0 if an empty array, 1 if one item array
*/
export function findMinInterval(xValues: number[]): number {
const valuesLength = xValues.length - 1;
if (valuesLength < 0) {
const valuesLength = xValues.length;
if (valuesLength <= 0) {
return 0;
}
if (valuesLength === 0) {
if (valuesLength === 1) {
return 1;
}
const sortedValues = xValues.slice().sort(compareByValueAsc);
let i;
let minInterval = null;
for (i = 0; i < valuesLength; i++) {
let minInterval = Math.abs(sortedValues[1] - sortedValues[0]);
for (i = 1; i < valuesLength - 1; i++) {
const current = sortedValues[i];
const next = sortedValues[i + 1];
const interval = Math.abs(next - current);
if (minInterval === null) {
minInterval = interval;
} else {
minInterval = Math.min(minInterval, interval);
}
minInterval = Math.min(minInterval, interval);
}
return minInterval === null ? 0 : minInterval;
return minInterval;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/lib/series/specs.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export interface SeriesScales {
* The x axis scale type
* @default ScaleType.Ordinal
*/
xScaleType: ScaleType;
xScaleType: ScaleType.Ordinal | ScaleType.Linear | ScaleType.Time;
/**
* The y axis scale type
* @default ScaleType.Linear
Expand Down
24 changes: 15 additions & 9 deletions wiki/overview.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,29 +129,35 @@ export interface SeriesSpec {
}

export interface SeriesAccessors {
/* The field name of the x value on Datum object */
/** The field name of the x value on Datum object */
xAccessor: Accessor;
/* An array of field names one per y metric value */
/** An array of field names one per y metric value */
yAccessors: Accessor[];
/* An array of fields thats indicates the datum series membership */
/** An array of fields thats indicates the datum series membership */
splitSeriesAccessors?: Accessor[];
/* An array of fields thats indicates the stack membership */
/** An array of fields thats indicates the stack membership */
stackAccessors?: Accessor[];
/* An optional array of field name thats indicates the stack membership */
/** An optional array of field name thats indicates the stack membership */
colorAccessors?: Accessor[];
}

export interface SeriesScales {
/* The x axis scale type */
xScaleType: ScaleType;
/* The y axis scale type */
/**
* The x axis scale type
* @default ScaleType.Ordinal
*/
xScaleType: ScaleType.Ordinal | ScaleType.Linear | ScaleType.Time;
/**
* The y axis scale type
* @default ScaleType.Linear
*/
yScaleType: ScaleContinuousType;
/** if true, the min y value is set to the minimum domain value, 0 otherwise */
yScaleToDataExtent: boolean;
}
```

A `BarSeriesSpec` for example is the following union type:
A `BarSeriesSpec` for example is the following intersection type:

```ts
export type BarSeriesSpec = SeriesSpec &
Expand Down

0 comments on commit 59c3b70

Please sign in to comment.