From 7190f6c9705a7c69b0851b356dd9373b90013a79 Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Wed, 5 Feb 2020 11:35:08 -0600 Subject: [PATCH 1/2] fix: empty domain error for ordinal x scale --- .../xy_chart/domains/x_domain.test.ts | 41 +++++++++++++++++++ src/chart_types/xy_chart/domains/x_domain.ts | 5 +++ 2 files changed, 46 insertions(+) diff --git a/src/chart_types/xy_chart/domains/x_domain.test.ts b/src/chart_types/xy_chart/domains/x_domain.test.ts index ede1a8bcea..322584a0e7 100644 --- a/src/chart_types/xy_chart/domains/x_domain.test.ts +++ b/src/chart_types/xy_chart/domains/x_domain.test.ts @@ -168,6 +168,47 @@ describe('X Domain', () => { }); }); + test('Should return empty domain when there is no data', () => { + const ds1: BasicSeriesSpec = { + chartType: ChartTypes.XYAxis, + specType: SpecTypes.Series, + id: 'ds1', + groupId: 'g1', + seriesType: SeriesTypes.Line, + xAccessor: 'x', + yAccessors: ['y'], + xScaleType: ScaleType.Ordinal, + yScaleType: ScaleType.Linear, + yScaleToDataExtent: false, + data: [], + }; + const ds2: BasicSeriesSpec = { + chartType: ChartTypes.XYAxis, + specType: SpecTypes.Series, + id: 'ds2', + groupId: 'g1', + seriesType: SeriesTypes.Line, + xAccessor: 'x', + yAccessors: ['y'], + xScaleType: ScaleType.Ordinal, + yScaleType: ScaleType.Linear, + yScaleToDataExtent: false, + data: [], + }; + const specDataSeries: BasicSeriesSpec[] = [ds1, ds2]; + const { xValues } = getSplittedSeries(specDataSeries); + const mergedDomain = mergeXDomain( + [ + { + seriesType: SeriesTypes.Line, + xScaleType: ScaleType.Linear, + }, + ], + xValues, + ); + expect(mergedDomain.domain).toEqual([0, 0]); + }); + test('Should merge line series correctly', () => { const ds1: BasicSeriesSpec = { chartType: ChartTypes.XYAxis, diff --git a/src/chart_types/xy_chart/domains/x_domain.ts b/src/chart_types/xy_chart/domains/x_domain.ts index f2aef2bd2f..41a17f1615 100644 --- a/src/chart_types/xy_chart/domains/x_domain.ts +++ b/src/chart_types/xy_chart/domains/x_domain.ts @@ -93,6 +93,11 @@ export function mergeXDomain( minInterval = customMinInterval || computedMinInterval; } + // TODO: Check for empty data before computing domain + if (seriesXComputedDomains.length === 0) { + seriesXComputedDomains = [0, 0]; + } + return { type: 'xDomain', scaleType: mainXScaleType.scaleType, From c41c0efb3934cb578700572d41759b23b2f0ac79 Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Wed, 5 Feb 2020 12:03:14 -0600 Subject: [PATCH 2/2] chore: address Marco's wise comments --- .../xy_chart/domains/x_domain.test.ts | 41 ------------------- src/chart_types/xy_chart/domains/x_domain.ts | 5 --- src/utils/domain.test.ts | 11 +++++ src/utils/domain.ts | 5 +++ 4 files changed, 16 insertions(+), 46 deletions(-) diff --git a/src/chart_types/xy_chart/domains/x_domain.test.ts b/src/chart_types/xy_chart/domains/x_domain.test.ts index 322584a0e7..ede1a8bcea 100644 --- a/src/chart_types/xy_chart/domains/x_domain.test.ts +++ b/src/chart_types/xy_chart/domains/x_domain.test.ts @@ -168,47 +168,6 @@ describe('X Domain', () => { }); }); - test('Should return empty domain when there is no data', () => { - const ds1: BasicSeriesSpec = { - chartType: ChartTypes.XYAxis, - specType: SpecTypes.Series, - id: 'ds1', - groupId: 'g1', - seriesType: SeriesTypes.Line, - xAccessor: 'x', - yAccessors: ['y'], - xScaleType: ScaleType.Ordinal, - yScaleType: ScaleType.Linear, - yScaleToDataExtent: false, - data: [], - }; - const ds2: BasicSeriesSpec = { - chartType: ChartTypes.XYAxis, - specType: SpecTypes.Series, - id: 'ds2', - groupId: 'g1', - seriesType: SeriesTypes.Line, - xAccessor: 'x', - yAccessors: ['y'], - xScaleType: ScaleType.Ordinal, - yScaleType: ScaleType.Linear, - yScaleToDataExtent: false, - data: [], - }; - const specDataSeries: BasicSeriesSpec[] = [ds1, ds2]; - const { xValues } = getSplittedSeries(specDataSeries); - const mergedDomain = mergeXDomain( - [ - { - seriesType: SeriesTypes.Line, - xScaleType: ScaleType.Linear, - }, - ], - xValues, - ); - expect(mergedDomain.domain).toEqual([0, 0]); - }); - test('Should merge line series correctly', () => { const ds1: BasicSeriesSpec = { chartType: ChartTypes.XYAxis, diff --git a/src/chart_types/xy_chart/domains/x_domain.ts b/src/chart_types/xy_chart/domains/x_domain.ts index 41a17f1615..f2aef2bd2f 100644 --- a/src/chart_types/xy_chart/domains/x_domain.ts +++ b/src/chart_types/xy_chart/domains/x_domain.ts @@ -93,11 +93,6 @@ export function mergeXDomain( minInterval = customMinInterval || computedMinInterval; } - // TODO: Check for empty data before computing domain - if (seriesXComputedDomains.length === 0) { - seriesXComputedDomains = [0, 0]; - } - return { type: 'xDomain', scaleType: mainXScaleType.scaleType, diff --git a/src/utils/domain.test.ts b/src/utils/domain.test.ts index bcbab252a5..d505a3e260 100644 --- a/src/utils/domain.test.ts +++ b/src/utils/domain.test.ts @@ -7,6 +7,17 @@ import { } from './domain'; describe('utils/domain', () => { + test('should return [0] domain if no data', () => { + const data: any[] = []; + const accessor: AccessorFn = (datum: any) => datum.x; + const isSorted = true; + const removeNull = true; + + const ordinalDataDomain = computeOrdinalDataDomain(data, accessor, isSorted, removeNull); + + expect(ordinalDataDomain).toEqual([0]); + }); + test('should compute ordinal data domain: sort & remove nulls', () => { const data = [{ x: 'd' }, { x: 'a' }, { x: null }, { x: 'b' }]; const accessor: AccessorFn = (datum: any) => datum.x; diff --git a/src/utils/domain.ts b/src/utils/domain.ts index f4d9b5c6bc..c71224b15f 100644 --- a/src/utils/domain.ts +++ b/src/utils/domain.ts @@ -36,6 +36,11 @@ export function computeOrdinalDataDomain( sorted?: boolean, removeNull?: boolean, ): string[] | number[] { + // TODO: Check for empty data before computing domain + if (data.length === 0) { + return [0]; + } + const domain = data.map(accessor).filter((d) => (removeNull ? d !== null : true)); const uniqueValues = [...new Set(domain)]; return sorted