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

fix(xy): stacked polarity #1502

Merged
merged 29 commits into from
Dec 9, 2021
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
6637aaf
fix: stacked polarity
nickofthyme Nov 23, 2021
de17519
refactor: stacking logic to group by x values
nickofthyme Dec 3, 2021
5b08ee4
feat: add diverging support for all offset types
nickofthyme Dec 8, 2021
0dcb91a
Merge branch 'master' into fix-stacked-polarity
nickofthyme Dec 8, 2021
56249bc
fix: block using y0 accessors with split accessors
nickofthyme Dec 8, 2021
ef38163
fix: add bypass for non-mixed polarity data
nickofthyme Dec 8, 2021
6f1cdb9
fix: remove filtered series from stack logic
nickofthyme Dec 8, 2021
c8eefea
fix: percentage stack offset to work for mixed polarity
nickofthyme Dec 8, 2021
96797fe
fix: wiggle for negative values
nickofthyme Dec 8, 2021
cf7ae39
fix: cleanup types for offset functions
nickofthyme Dec 8, 2021
a9e7486
test: add vrt for all offset options and polarity cases
nickofthyme Dec 8, 2021
0f40afc
fix: diverging logic for 0 case
nickofthyme Dec 8, 2021
fde8d5a
refactor: offset values to include y0 for fitting purposes
nickofthyme Dec 8, 2021
c1cf8ad
refactor: ignore y0Accessors when used with stackAccessors at the dat…
nickofthyme Dec 8, 2021
25d11af
fix: apply percentage domain logic only when stacked
nickofthyme Dec 8, 2021
8a1aef2
fix: remove y0 points in disallowed banded stack area charts
nickofthyme Dec 8, 2021
e1034d8
fix: series toggle logic with stacked offsets
nickofthyme Dec 8, 2021
0a58d76
fix: percent offset stopping after sum of zero
nickofthyme Dec 8, 2021
1b0a6a1
test: update vrt screenshots for mixes stories
nickofthyme Dec 8, 2021
ef547ea
fix: stacking order and stack value logic
nickofthyme Dec 8, 2021
7e9149a
test: add tests for found and fixed edge cases
nickofthyme Dec 8, 2021
075527b
test: prune stale vrt screenshots
nickofthyme Dec 8, 2021
d4c0196
fix: linting error
nickofthyme Dec 8, 2021
e916c6d
test: fix failing tests
nickofthyme Dec 9, 2021
9eb5bf4
feat: add 0 baseline datum
nickofthyme Dec 9, 2021
73a9e3c
refactor: rename banded spec check to be consistent throughout
nickofthyme Dec 9, 2021
ff2f537
test: update vrt screenshots with added baseline datum
nickofthyme Dec 9, 2021
f06db8a
chore: add warning when using area charts on mixed polarity datasets
nickofthyme Dec 9, 2021
edcb883
test: fix failing tests
nickofthyme Dec 9, 2021
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
2 changes: 1 addition & 1 deletion NOTICE.txt
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ THIS SOFTWARE.

---

This product includes code that is adapted from d3-array@3.0.4 and d3-scale@4.0.2,
This product includes code that is adapted from d3-shape@3.0.1, d3-array@3.0.4 and d3-scale@4.0.2,
which are both available under a "ISC" license.

ISC License
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
12 changes: 11 additions & 1 deletion integration/tests/mixed_stories.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* Side Public License, v 1.
*/

import { Fit } from '../../packages/charts/src';
import { Fit, StackMode, SeriesType } from '../../packages/charts/src';
import { common } from '../page_objects';

describe('Mixed series stories', () => {
Expand Down Expand Up @@ -195,4 +195,14 @@ describe('Mixed series stories', () => {
});
});
});

describe.each(Object.values(StackMode))('Stack mode - %s', (mode) => {
describe.each(['Mixed', 'Positive', 'Negative'])('Polarity - %s', (polarity) => {
it.each([SeriesType.Bar, SeriesType.Area])('should display correct stacking for %s series', async (type) => {
await common.expectChartAtUrlToMatchScreenshot(
`http://localhost:9001/?path=/story/mixed-charts---polarized-stacked&globals=theme:light&knob-stacked=true&knob-data polarity=${polarity}&knob-custom domain=false&knob-stackMode=${mode}&knob-SeriesType=${type}`,
);
});
});
});
});
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@
"@types/d3-collection": "^1.0.8",
"@types/d3-interpolate": "^1.3.1",
"@types/d3-scale": "^2.1.1",
"@types/d3-shape": "^1.3.1",
"@types/d3-shape": "^2.0.0",
"@types/enzyme": "^3.9.0",
"@types/enzyme-adapter-react-16": "^1.0.5",
"@types/expect-puppeteer": "^4.4.5",
Expand Down
2 changes: 1 addition & 1 deletion packages/charts/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
"d3-collection": "^1.0.7",
"d3-interpolate": "^1.4.0",
"d3-scale": "^1.0.7",
"d3-shape": "^1.3.4",
"d3-shape": "^2.0.0",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what was the issue on upgrading to the latest version?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The jest tests were throwing an error similar to https://stackoverflow.com/questions/49263429/jest-gives-an-error-syntaxerror-unexpected-token-export. I tried to fix it but couldn't see any difference between the 2 so I gave up for now.

"prop-types": "^15.7.2",
"re-reselect": "^3.4.0",
"react-redux": "^7.1.0",
Expand Down
4 changes: 2 additions & 2 deletions packages/charts/src/chart_types/xy_chart/domains/y_domain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,14 @@ function mergeYDomainForGroup(
const dataSeries = [...stacked, ...nonStacked];
if (dataSeries.length === 0) return null;

const [{ stackMode, spec }] = dataSeries;
const [{ isStacked, stackMode, spec }] = dataSeries;
const groupId = getSpecDomainGroupId(spec);
const { customDomain, type, nice, desiredTickCount } = yScaleConfig[groupId];
const newCustomDomain: YDomainRange = customDomain ? { ...customDomain } : { min: NaN, max: NaN };
const { paddingUnit, padding, constrainPadding } = newCustomDomain;

let mergedDomain: ContinuousDomain;
if (stackMode === StackMode.Percentage) {
if (isStacked && stackMode === StackMode.Percentage) {
mergedDomain = computeContinuousDataDomain([0, 1], type, customDomain);
} else {
nickofthyme marked this conversation as resolved.
Show resolved Hide resolved
const stackedDomain = computeYDomain(stacked, hasZeroBaselineSpecs, type, newCustomDomain);
Expand Down
3 changes: 2 additions & 1 deletion packages/charts/src/chart_types/xy_chart/legend/legend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { mergePartial, Rotation } from '../../../utils/common';
import { BandedAccessorType } from '../../../utils/geometry';
import { getLegendCompareFn } from '../../../utils/series_sort';
import { PointStyle, Theme } from '../../../utils/themes/theme';
import { isStackedSpec } from '../domains/y_domain';
import { getXScaleTypeFromSpec } from '../scales/get_api_scales';
import { getAxesSpecForSpecId, getSpecsById } from '../state/utils/spec';
import { LastValues } from '../state/utils/types';
Expand Down Expand Up @@ -111,7 +112,7 @@ export function computeLegend(

dataSeries.forEach((series) => {
const { specId, yAccessor } = series;
const banded = isDataSeriesBanded(series);
const banded = isDataSeriesBanded(series) && !isStackedSpec(series.spec, true);
const key = getSeriesKey(series, series.groupId);
const spec = getSpecsById<BasicSeriesSpec>(specs, specId);
const dataSeriesKey = getSeriesKey(
Expand Down
7 changes: 4 additions & 3 deletions packages/charts/src/chart_types/xy_chart/rendering/area.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export function renderArea(
areaGeometry: AreaGeometry;
indexedGeometryMap: IndexedGeometryMap;
} {
const isBandChart = hasY0Accessors && !isStacked;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we reuse isDataSeriesBanded here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const y1Fn = getY1ScaledValueFn(yScale);
const y0Fn = getY0ScaledValueFn(yScale);
const definedFn = isYValueDefinedFn(yScale, xScale);
Expand All @@ -57,15 +58,15 @@ export function renderArea(
.y1(y1Fn)
.y0(y0Fn)
.defined((datum) => {
return definedFn(datum, y1DatumAccessor) && (hasY0Accessors ? definedFn(datum, y0DatumAccessor) : true);
return definedFn(datum, y1DatumAccessor) && (isBandChart ? definedFn(datum, y0DatumAccessor) : true);
})
.curve(getCurveFactory(curve));

// TODO we can probably avoid this function call if no fit function is applied.
const clippedRanges = getClippedRanges(dataSeries.data, xScale, xScaleOffset);

const lines: string[] = [];
const y0Line = hasY0Accessors && pathGenerator.lineY0()(dataSeries.data);
const y0Line = isBandChart && pathGenerator.lineY0()(dataSeries.data);
const y1Line = pathGenerator.lineY1()(dataSeries.data);
if (y1Line) lines.push(y1Line);
if (y0Line) lines.push(y0Line);
Expand All @@ -78,7 +79,7 @@ export function renderArea(
panel,
color,
style.point,
hasY0Accessors,
isBandChart,
markSizeOptions,
false,
pointStyleAccessor,
Expand Down
8 changes: 4 additions & 4 deletions packages/charts/src/chart_types/xy_chart/rendering/points.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export function renderPoints(
panel: Dimensions,
color: Color,
pointStyle: PointStyle,
hasY0Accessors: boolean,
isBandChart: boolean,
markSizeOptions: MarkSizeOptions,
useSpatialIndex: boolean,
styleAccessor?: PointStyleAccessor,
Expand Down Expand Up @@ -66,12 +66,12 @@ export function renderPoints(
if (Number.isNaN(x)) return acc;

const points: PointGeometry[] = [];
const yDatumKeyNames: Array<keyof Omit<FilledValues, 'x'>> = hasY0Accessors ? ['y0', 'y1'] : ['y1'];
const yDatumKeyNames: Array<keyof Omit<FilledValues, 'x'>> = isBandChart ? ['y0', 'y1'] : ['y1'];

yDatumKeyNames.forEach((yDatumKeyName, keyIndex) => {
const valueAccessor = getYDatumValueFn(yDatumKeyName);
const y = yDatumKeyName === 'y1' ? y1Fn(datum) : y0Fn(datum);
const originalY = getDatumYValue(datum, keyIndex === 0, hasY0Accessors, dataSeries.stackMode);
const originalY = getDatumYValue(datum, keyIndex === 0, isBandChart, dataSeries.stackMode);
const seriesIdentifier: XYChartSeriesIdentifier = {
key: dataSeries.key,
specId: dataSeries.specId,
Expand All @@ -98,7 +98,7 @@ export function renderPoints(
x: xValue,
y: originalY,
mark,
accessor: hasY0Accessors && keyIndex === 0 ? BandedAccessorType.Y0 : BandedAccessorType.Y1,
accessor: isBandChart && keyIndex === 0 ? BandedAccessorType.Y0 : BandedAccessorType.Y1,
datum: datum.datum,
},
transform: {
Expand Down
7 changes: 6 additions & 1 deletion packages/charts/src/chart_types/xy_chart/tooltip/tooltip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { TooltipValue } from '../../../specs';
import { getAccessorFormatLabel } from '../../../utils/accessor';
import { isDefined } from '../../../utils/common';
import { BandedAccessorType, IndexedGeometry } from '../../../utils/geometry';
import { isStackedSpec } from '../domains/y_domain';
import { defaultTickFormatter } from '../utils/axis_utils';
import { getSeriesName } from '../utils/series';
import {
Expand Down Expand Up @@ -66,7 +67,11 @@ export function formatTooltip(
): TooltipValue {
let label = getSeriesName(seriesIdentifier, hasSingleSeries, true, spec);

if (isBandedSpec(spec.y0Accessors) && (isAreaSeriesSpec(spec) || isBarSeriesSpec(spec))) {
if (
isBandedSpec(spec.y0Accessors) &&
!isStackedSpec(spec, true) &&
(isAreaSeriesSpec(spec) || isBarSeriesSpec(spec))
) {
const { y0AccessorFormat = Y0_ACCESSOR_POSTFIX, y1AccessorFormat = Y1_ACCESSOR_POSTFIX } = spec;
const formatter = accessor === BandedAccessorType.Y0 ? y0AccessorFormat : y1AccessorFormat;
label = getAccessorFormatLabel(formatter, label);
Expand Down
202 changes: 202 additions & 0 deletions packages/charts/src/chart_types/xy_chart/utils/diverging_offsets.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,202 @@
/* eslint-disable header/header, no-param-reassign */

/**
* @notice
* This product includes code that is adapted from d3-shape@3.0.1,
* which is available under a "ISC" license.
*
* ISC License
*
* Copyright 2010-2021 Mike Bostock
* Permission to use, copy, modify, and/or distribute this software for any purpose
* with or without fee is hereby granted, provided that the above copyright notice
* and this permission notice appear in all copies.

* THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH
* REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
* FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT,
* INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS
* OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
* TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF
* THIS SOFTWARE.
*/

import { Series, SeriesPoint } from 'd3-shape';

import { SeriesKey } from '../../../common/series_id';
import { DataSeriesDatum } from './series';

type XValue = string | number;
type SeriesValueMap = Map<SeriesKey, DataSeriesDatum>;

/** @internal */
export type XValueMap = Map<XValue, SeriesValueMap>;
/** @internal */
export type XValueSeriesDatum = [XValue, SeriesValueMap];

/**
* Computes required wiggle offset for each x value __WITHOUT__ mutations
*/
function getWiggleOffsets<K = string>(series: Series<XValueSeriesDatum, K>, order: number[]): number[] {
const offsets = [];
let y, j;
for (y = 0, j = 1; j < series[order[0]].length; ++j) {
let i, s1, s2;
for (i = 0, s1 = 0, s2 = 0; i < series.length; ++i) {
// @ts-ignore - d3-shape type here is inaccurate
const si = series[order[i]] as SeriesPoint<XValueSeriesDatum>[];
const sij0 = si[j][1] || 0;
const sij1 = si[j - 1][1] || 0;
let s3 = (sij0 - sij1) / 2;

for (let k = 0; k < i; ++k) {
// @ts-ignore - d3-shape type here is inaccurate
const sk = series[order[k]] as SeriesPoint<XValueSeriesDatum>[];
const skj0 = sk[j][1] || 0;
const skj1 = sk[j - 1][1] || 0;
s3 += skj0 - skj1;
}
s1 += sij0;
s2 += s3 * sij0;
}

offsets.push(y);
if (s1) y -= s2 / s1;
}
offsets.push(y);
return offsets;
}

/** @internal */
const divergingOffset = (isSilhouette = false) => {
return function <K = 'string'>(series: Series<XValueSeriesDatum, K>, order: number[]): void {
const n = series.length;
if (!(n > 0)) return;
for (let i, j = 0, sumYn, sumYp, yp, yn = 0, s0 = series[order[0]], m = s0.length; j < m; ++j) {
// sum negative values per x before to maintain original sort for negative values
for (yn = 0, sumYn = 0, sumYp = 0, i = 0; i < n; ++i) {
// @ts-ignore - d3-shape type here is inaccurate
const d = series[order[i]][j] as SeriesPoint<XValueSeriesDatum>;
const dy = d[1] - d[0];
if (dy < 0) {
sumYn += Math.abs(d[1]) || 0;
yn += dy;
} else {
sumYp += d[1] || 0;
}
}

const silhouetteOffset = sumYp / 2 - sumYn / 2;
const offset = isSilhouette ? -silhouetteOffset : 0;
yn += offset;

for (yp = offset, i = 0; i < n; ++i) {
// @ts-ignore - d3-shape type here is inaccurate
const d = series[order[i]][j] as SeriesPoint<XValueSeriesDatum>;
const dy = d[1] - d[0];
if (dy >= 0) {
d[0] = yp;
d[1] = yp += dy;
} else {
d[1] = yn;
d[0] = yn -= dy;
}
}
}
};
};

/**
* Stacked offset function with diverging polarity offset
* @internal
*/
export const diverging = divergingOffset();
/**
* Stacked Silhouette offset function with diverging polarity offset
* @internal
*/
export const divergingSilhouette = divergingOffset(true);

/**
* Stacked Wiggle offset function to account for diverging offset
* @internal
*/
export function divergingWiggle<K = 'string'>(series: Series<XValueSeriesDatum, K>, order: number[]): void {
const n = series.length;
const s0 = series[order[0]];
const m = s0.length;
if (!(n > 0) || !(m > 0)) return diverging(series, order);

const offsets = getWiggleOffsets(series, order);

for (let i, j = 0, sumYn, yp, yn = 0; j < m; ++j) {
// sum negative values per x before to maintain original sort for negative values
for (i = 0, yn = 0, sumYn = 0; i < n; ++i) {
// @ts-ignore - d3-shape type here is inaccurate
const d = series[order[i]][j] as SeriesPoint<XValueSeriesDatum>;
if (d[1] - d[0] < 0) {
sumYn += Math.abs(d[1]) || 0;
}
}

const offset = offsets[j];
yn += offset;

for (yp = offset + sumYn, yn = offset, i = 0; i < n; ++i) {
// @ts-ignore - d3-shape type here is inaccurate
const d = series[order[i]][j] as SeriesPoint<XValueSeriesDatum>;
const dy = d[1] - d[0];
if (dy >= 0) {
d[0] = yp;
d[1] = yp += dy;
} else {
d[1] = yn;
d[0] = yn -= dy;
}
}
}
}

/**
* Stacked Percentage offset function with diverging polarity offset
* Treats percentage as participation for mixed polarity data
* @internal
*/
export function divergingPercentage<K = 'string'>(series: Series<XValueSeriesDatum, K>, order: number[]): void {
const n = series.length;
if (!(n > 0)) return;
for (let i, j = 0, sumYn, sumYp; j < series[0].length; ++j) {
for (sumYn = sumYp = i = 0; i < n; ++i) {
// @ts-ignore - d3-shape type here is inaccurate
const d = series[order[i]][j] as SeriesPoint<XValueSeriesDatum>;
if (d[1] - d[0] < 0) {
sumYn += Math.abs(d[1]) || 0;
} else {
sumYp += d[1] || 0;
}
}

const sumY = sumYn + sumYp;
if (sumY === 0) continue; // must not return, else loop will stop

let yp = sumYn / sumY;
let yn = 0;

for (i = 0; i < n; ++i) {
// @ts-ignore - d3-shape type here is inaccurate
const d = series[order[i]][j] as SeriesPoint<XValueSeriesDatum>;
const dy = d[1] - d[0];
const participation = Math.abs(dy / sumY);

if (dy >= 0) {
d[0] = yp;
d[1] = yp += participation;
} else {
d[0] = yn;
d[1] = yn += participation;
}
}
}
}

/* eslint-enable header/header, no-param-reassign */
Loading