Skip to content

Commit

Permalink
fix(rendering): fix rendering values <= 0 on log scale (opensearch-pr…
Browse files Browse the repository at this point in the history 
…oject#114)

Instead of throwing errors or warnings, if the user is using a log scale with a dataset with 0
values, we will hide them on the rendering of line and area charts. This fix also the defined area
of the chart

fix opensearch-project#112, fix opensearch-project#63
  • Loading branch information
@markov00
markov00 committed Mar 26, 2019
1 parent 306d0b8 commit 2b79877
Show file tree
Hide file tree
Showing 8 changed files with 2,015 additions and 42 deletions.
764 changes: 764 additions & 0 deletions packages/osd-charts/src/lib/series/rendering.areas.test.ts
View file

Large diffs are not rendered by default.

10 changes: 5 additions & 5 deletions packages/osd-charts/src/lib/series/rendering.bars.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const SPEC_ID = getSpecId('spec_1');
const GROUP_ID = getGroupId('group_1');

describe('Rendering bars', () => {
describe('Single series barchart - ordinal', () => {
describe('Single series bar chart - ordinal', () => {
const barSeriesSpec: BarSeriesSpec = {
id: SPEC_ID,
groupId: GROUP_ID,
Expand Down Expand Up @@ -70,7 +70,7 @@ describe('Rendering bars', () => {
});
});
});
describe('Multi series barchart - ordinal', () => {
describe('Multi series bar chart - ordinal', () => {
const spec1Id = getSpecId('bar1');
const spec2Id = getSpecId('bar2');
const barSeriesSpec1: BarSeriesSpec = {
Expand Down Expand Up @@ -191,7 +191,7 @@ describe('Rendering bars', () => {
});
});
});
describe('Single series barchart - lineaar', () => {
describe('Single series bar chart - lineaar', () => {
const barSeriesSpec: BarSeriesSpec = {
id: SPEC_ID,
groupId: GROUP_ID,
Expand Down Expand Up @@ -253,7 +253,7 @@ describe('Rendering bars', () => {
});
});
});
describe('Multi series barchart - linear', () => {
describe('Multi series bar chart - linear', () => {
const spec1Id = getSpecId('bar1');
const spec2Id = getSpecId('bar2');
const barSeriesSpec1: BarSeriesSpec = {
Expand Down Expand Up @@ -374,7 +374,7 @@ describe('Rendering bars', () => {
});
});
});
describe('Multi series barchart - time', () => {
describe('Multi series bar chart - time', () => {
const spec1Id = getSpecId('bar1');
const spec2Id = getSpecId('bar2');
const barSeriesSpec1: BarSeriesSpec = {
Expand Down
83 changes: 77 additions & 6 deletions ...s/src/lib/series/rendering.points.test.ts → ...ts/src/lib/series/rendering.lines.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const SPEC_ID = getSpecId('spec_1');
const GROUP_ID = getGroupId('group_1');

describe('Rendering points - line', () => {
describe('Single series point chart - ordinal', () => {
describe('Single series line chart - ordinal', () => {
const pointSeriesSpec: LineSeriesSpec = {
id: SPEC_ID,
groupId: GROUP_ID,
Expand Down Expand Up @@ -88,7 +88,7 @@ describe('Rendering points - line', () => {
expect(indexedGeometries.size).toEqual(points.length);
});
});
describe('Multi series pointchart - ordinal', () => {
describe('Multi series line chart - ordinal', () => {
const spec1Id = getSpecId('point1');
const spec2Id = getSpecId('point2');
const pointSeriesSpec1: LineSeriesSpec = {
Expand Down Expand Up @@ -238,7 +238,7 @@ describe('Rendering points - line', () => {
expect(indexedGeometries.size).toEqual(points.length);
});
});
describe('Single series pointchart - linear', () => {
describe('Single series line chart - linear', () => {
const pointSeriesSpec: LineSeriesSpec = {
id: SPEC_ID,
groupId: GROUP_ID,
Expand Down Expand Up @@ -316,7 +316,7 @@ describe('Rendering points - line', () => {
expect(indexedGeometries.size).toEqual(points.length);
});
});
describe('Multi series pointchart - linear', () => {
describe('Multi series line chart - linear', () => {
const spec1Id = getSpecId('point1');
const spec2Id = getSpecId('point2');
const pointSeriesSpec1: LineSeriesSpec = {
Expand Down Expand Up @@ -465,7 +465,7 @@ describe('Rendering points - line', () => {
expect(indexedGeometries.size).toEqual(points.length);
});
});
describe('Single series pointchart - time', () => {
describe('Single series line chart - time', () => {
const pointSeriesSpec: LineSeriesSpec = {
id: SPEC_ID,
groupId: GROUP_ID,
Expand Down Expand Up @@ -543,7 +543,7 @@ describe('Rendering points - line', () => {
expect(indexedGeometries.size).toEqual(points.length);
});
});
describe('Multi series pointchart - time', () => {
describe('Multi series line chart - time', () => {
const spec1Id = getSpecId('point1');
const spec2Id = getSpecId('point2');
const pointSeriesSpec1: LineSeriesSpec = {
Expand Down Expand Up @@ -679,4 +679,75 @@ describe('Rendering points - line', () => {
expect(indexedGeometries.size).toEqual(points.length);
});
});
describe('Single series line chart - y log', () => {
const pointSeriesSpec: LineSeriesSpec = {
id: SPEC_ID,
groupId: GROUP_ID,
seriesType: 'line',
yScaleToDataExtent: false,
data: [[0, 10], [1, 5], [2, null], [3, 5], [4, 5], [5, 0], [6, 10], [7, 10], [8, 10]],
xAccessor: 0,
yAccessors: [1],
xScaleType: ScaleType.Linear,
yScaleType: ScaleType.Log,
};
const pointSeriesMap = new Map();
pointSeriesMap.set(SPEC_ID, pointSeriesSpec);
const pointSeriesDomains = computeSeriesDomains(pointSeriesMap, new Map());
const xScale = computeXScale(pointSeriesDomains.xDomain, pointSeriesMap.size, 0, 90);
const yScales = computeYScales(pointSeriesDomains.yDomain, 100, 0);

let renderedLine: {
lineGeometry: LineGeometry;
indexedGeometries: Map<any, IndexedGeometry[]>;
};

beforeEach(() => {
renderedLine = renderLine(
0, // not applied any shift, renderGeometries applies it only with mixed charts
pointSeriesDomains.formattedDataSeries.nonStacked[0].dataSeries[0].data,
xScale,
yScales.get(GROUP_ID)!,
'red',
CurveType.LINEAR,
SPEC_ID,
[],
);
});
test('Can render a splitted line', () => {
// expect(renderedLine.lineGeometry.line).toBe('ss');
expect(renderedLine.lineGeometry.line.split('M').length - 1).toBe(3);
expect(renderedLine.lineGeometry.color).toBe('red');
expect(renderedLine.lineGeometry.geometryId.seriesKey).toEqual([]);
expect(renderedLine.lineGeometry.geometryId.specId).toEqual(SPEC_ID);
expect(renderedLine.lineGeometry.transform).toEqual({ x: 0, y: 0 });
});
test('Can render points', () => {
const {
lineGeometry: { points },
indexedGeometries,
} = renderedLine;
// all the points minus the undefined ones on a log scale
expect(points.length).toBe(7);
// all the points
expect(indexedGeometries.size).toEqual(9);
const nullIndexdGeometry = indexedGeometries.get(2);
expect(nullIndexdGeometry).toBeDefined();
expect(nullIndexdGeometry!.length).toBe(1);
// moved to the bottom of the chart
expect(nullIndexdGeometry![0].geom.y).toBe(100);
// 0 radius point
expect(nullIndexdGeometry![0].geom.width).toBe(0);
expect(nullIndexdGeometry![0].geom.height).toBe(0);

const zeroValueIndexdGeometry = indexedGeometries.get(5);
expect(zeroValueIndexdGeometry).toBeDefined();
expect(zeroValueIndexdGeometry!.length).toBe(1);
// moved to the bottom of the chart
expect(zeroValueIndexdGeometry![0].geom.y).toBe(100);
// 0 radius point
expect(zeroValueIndexdGeometry![0].geom.width).toBe(0);
expect(zeroValueIndexdGeometry![0].geom.height).toBe(0);
});
});
});
89 changes: 59 additions & 30 deletions packages/osd-charts/src/lib/series/rendering.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { area, line } from 'd3-shape';
import { mutableIndexedGeometryMapUpsert } from '../../state/utils';
import { SharedGeometryStyle } from '../themes/theme';
import { SpecId } from '../utils/ids';
import { isLogarithmicScale } from '../utils/scales/scale_continuous';
import { Scale, ScaleType } from '../utils/scales/scales';
import { CurveType, getCurveFactory } from './curves';
import { LegendItem } from './legend';
Expand Down Expand Up @@ -87,39 +88,56 @@ export function renderPoints(
indexedGeometries: Map<any, IndexedGeometry[]>;
} {
const indexedGeometries: Map<any, IndexedGeometry[]> = new Map();
const isLogScale = isLogarithmicScale(yScale);

const pointGeometries = dataset.map((datum) => {
const x = xScale.scale(datum.x);
const y = yScale.scale(datum.y1);
const indexedGeometry: IndexedGeometry = {
specId,
datum: datum.datum,
color,
seriesKey,
geom: {
x: x + shift,
y,
width: 10,
height: 10,
isPoint: true,
},
};
mutableIndexedGeometryMapUpsert(indexedGeometries, datum.x, indexedGeometry);
return {
x,
y,
color,
value: {
const pointGeometries = dataset.reduce(
(acc, datum) => {
const x = xScale.scale(datum.x);
let y;
let radius = 10;
const isHidden = datum.y1 === null || (isLogScale && datum.y1 <= 0);
// we fix 0 and negative values at y = 0
if (isHidden) {
y = yScale.range[0];
radius = 0;
} else {
y = yScale.scale(datum.y1);
}
const indexedGeometry: IndexedGeometry = {
specId,
datum: datum.datum,
color,
seriesKey,
},
transform: {
x: shift,
y: 0,
},
};
});
geom: {
x: x + shift,
y,
width: radius,
height: radius,
isPoint: true,
},
};
mutableIndexedGeometryMapUpsert(indexedGeometries, datum.x, indexedGeometry);
const pointGeometry: PointGeometry = {
x,
y,
color,
value: {
specId,
datum: datum.datum,
seriesKey,
},
transform: {
x: shift,
y: 0,
},
};
if (isHidden) {
return acc;
}
return [...acc, pointGeometry];
},
[] as PointGeometry[],
);
return {
pointGeometries,
indexedGeometries,
Expand Down Expand Up @@ -216,9 +234,12 @@ export function renderLine(
lineGeometry: LineGeometry;
indexedGeometries: Map<any, IndexedGeometry[]>;
} {
const isLogScale = isLogarithmicScale(yScale);

const pathGenerator = line<DataSeriesDatum>()
.x((datum: DataSeriesDatum) => xScale.scale(datum.x))
.y((datum: DataSeriesDatum) => yScale.scale(datum.y1))
.defined((datum: DataSeriesDatum) => datum.y1 !== null && !(isLogScale && datum.y1 <= 0))
.curve(getCurveFactory(curve));
const y = 0;
const x = shift;
Expand Down Expand Up @@ -263,10 +284,18 @@ export function renderArea(
areaGeometry: AreaGeometry;
indexedGeometries: Map<any, IndexedGeometry[]>;
} {
const isLogScale = isLogarithmicScale(yScale);

const pathGenerator = area<DataSeriesDatum>()
.x((datum: DataSeriesDatum) => xScale.scale(datum.x))
.y1((datum: DataSeriesDatum) => yScale.scale(datum.y1))
.y0((datum: DataSeriesDatum) => yScale.scale(datum.y0))
.y0((datum: DataSeriesDatum) => {
if (isLogScale && datum.y0 <= 0) {
return yScale.range[0];
}
return yScale.scale(datum.y0);
})
.defined((datum: DataSeriesDatum) => datum.y1 !== null && !(isLogScale && datum.y1 <= 0))
.curve(getCurveFactory(curve));
const { lineGeometry, indexedGeometries } = renderLine(
shift,
Expand Down
Loading

0 comments on commit 2b79877

Please sign in to comment.