Skip to content

Commit

Permalink
feat: order ordinal values by sum (opensearch-project#814)
Browse files Browse the repository at this point in the history
  • Loading branch information
nickofthyme authored Sep 12, 2020
1 parent 79d0e4f commit 45ea0c6
Show file tree
Hide file tree
Showing 12 changed files with 436 additions and 76 deletions.
31 changes: 31 additions & 0 deletions packages/osd-charts/api/charts.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,17 @@ export type BasicListener = () => undefined | void;
// @public (undocumented)
export type BasicSeriesSpec = SeriesSpec & SeriesAccessors & SeriesScales;

// Warning: (ae-missing-release-tag) "BinAgg" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
//
// @public
export const BinAgg: Readonly<{
Sum: "sum";
None: "none";
}>;

// @public (undocumented)
export type BinAgg = $Values<typeof BinAgg>;

// Warning: (ae-missing-release-tag) "BrushAxis" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
//
// @public (undocumented)
Expand Down Expand Up @@ -523,6 +534,17 @@ export const DEFAULT_TOOLTIP_TYPE: "vertical";
// @public (undocumented)
export type DefaultSettingsProps = 'id' | 'chartType' | 'specType' | 'rendering' | 'rotation' | 'resizeDebounce' | 'animateData' | 'showLegend' | 'debug' | 'tooltip' | 'showLegendExtra' | 'theme' | 'legendPosition' | 'hideDuplicateAxes' | 'brushAxis' | 'minBrushDelta' | 'externalPointerEvents';

// Warning: (ae-missing-release-tag) "Direction" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
//
// @public
export const Direction: Readonly<{
Ascending: "ascending";
Descending: "descending";
}>;

// @public (undocumented)
export type Direction = $Values<typeof Direction>;

// Warning: (ae-missing-release-tag) "DisplayValueSpec" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
//
// @public (undocumented)
Expand Down Expand Up @@ -920,6 +942,14 @@ export interface Opacity {
opacity: number;
}

// @public
export interface OrderBy {
// (undocumented)
binAgg?: BinAgg;
// (undocumented)
direction?: Direction;
}

// @public (undocumented)
export type PartialTheme = RecursivePartial<Theme>;

Expand Down Expand Up @@ -1332,6 +1362,7 @@ export interface SettingsSpec extends Spec {
onPointerUpdate?: PointerUpdateListener;
// (undocumented)
onRenderChange?: RenderChangeListener;
orderOrdinalBinsBy?: OrderBy;
// (undocumented)
pointBuffer?: MarkBuffer;
// (undocumented)
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ describe('Legend stories', () => {
const action = async () => await common.moveMouseRelativeToDOMElement({ left: 30, top: 10 }, '.echLegendItem');
await common.expectChartAtUrlToMatchScreenshot('http://localhost:9001/?path=/story/legend--actions', {
action,
delay: 200, // needed for icon to load
delay: 500, // needed for icon to load
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@
*/

import { ChartTypes } from '../..';
import { MockSeriesSpecs } from '../../../mocks/specs';
import { ScaleType } from '../../../scales/constants';
import { SpecTypes } from '../../../specs/constants';
import { SpecTypes, Direction, BinAgg } from '../../../specs/constants';
import { getDataSeriesBySpecId } from '../utils/series';
import { BasicSeriesSpec, SeriesTypes } from '../utils/specs';
import { convertXScaleTypes, findMinInterval, mergeXDomain } from './x_domain';
Expand Down Expand Up @@ -837,4 +838,97 @@ describe('X Domain', () => {
expect(attemptToMerge).toThrowError(expectedError);
});
});

describe('orderOrdinalBinsBySum', () => {
const ordinalSpecs = MockSeriesSpecs.fromPartialSpecs([
{
id: 'ordinal1',
seriesType: SeriesTypes.Bar,
xScaleType: ScaleType.Ordinal,
data: [
{ x: 'a', y: 2 },
{ x: 'b', y: 4 },
{ x: 'c', y: 8 },
{ x: 'd', y: 6 },
],
},
{
id: 'ordinal2',
seriesType: SeriesTypes.Bar,
xScaleType: ScaleType.Ordinal,
data: [
{ x: 'a', y: 4 },
{ x: 'b', y: 8 },
{ x: 'c', y: 16 },
{ x: 'd', y: 12 },
],
},
]);

const linearSpecs = MockSeriesSpecs.fromPartialSpecs([
{
id: 'linear1',
seriesType: SeriesTypes.Bar,
xScaleType: ScaleType.Linear,
data: [
{ x: 1, y: 2 },
{ x: 2, y: 4 },
{ x: 3, y: 8 },
{ x: 4, y: 6 },
],
},
{
id: 'linear2',
seriesType: SeriesTypes.Bar,
xScaleType: ScaleType.Linear,
data: [
{ x: 1, y: 4 },
{ x: 2, y: 8 },
{ x: 3, y: 16 },
{ x: 4, y: 12 },
],
},
]);

it('should sort ordinal xValues by descending sum by default', () => {
const { xValues } = getDataSeriesBySpecId(ordinalSpecs, [], {});
expect(xValues).toEqual(new Set(['c', 'd', 'b', 'a']));
});

it('should sort ordinal xValues by descending sum', () => {
const { xValues } = getDataSeriesBySpecId(ordinalSpecs, [], {
binAgg: BinAgg.None,
direction: Direction.Descending,
});
expect(xValues).toEqual(new Set(['c', 'd', 'b', 'a']));
});

it('should sort ordinal xValues by ascending sum', () => {
const { xValues } = getDataSeriesBySpecId(ordinalSpecs, [], {
binAgg: BinAgg.None,
direction: Direction.Ascending,
});
expect(xValues).toEqual(new Set(['a', 'b', 'd', 'c']));
});

it('should NOT sort ordinal xValues sum', () => {
const { xValues } = getDataSeriesBySpecId(ordinalSpecs, [], undefined);
expect(xValues).toEqual(new Set(['a', 'b', 'c', 'd']));
});

it('should NOT sort ordinal xValues sum when undefined', () => {
const { xValues } = getDataSeriesBySpecId(ordinalSpecs, [], {
binAgg: BinAgg.None,
direction: Direction.Descending,
});
expect(xValues).toEqual(new Set(['a', 'b', 'c', 'd']));
});

it('should NOT sort linear xValue by descending sum', () => {
const { xValues } = getDataSeriesBySpecId(linearSpecs, [], {
direction: Direction.Descending,
});
expect(xValues).toEqual(new Set([1, 2, 3, 4]));
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export const computeSeriesDomainsSelector = createCachedSelector(
customYDomainsByGroupId,
deselectedDataSeries,
settingsSpec.xDomain,
settingsSpec.orderOrdinalBinsBy,
// @ts-ignore blind sort option for vislib
settingsSpec.enableVislibSeriesSort,
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import { SeriesKey, SeriesIdentifier } from '../../../../commons/series_id';
import { Scale } from '../../../../scales';
import { ScaleType } from '../../../../scales/constants';
import { OrderBy } from '../../../../specs/settings';
import { mergePartial, Rotation, Color, isUniqueArray } from '../../../../utils/commons';
import { CurveType } from '../../../../utils/curves';
import { Dimensions } from '../../../../utils/dimensions';
Expand Down Expand Up @@ -204,14 +205,15 @@ export function computeSeriesDomains(
customYDomainsByGroupId: Map<GroupId, YDomainRange> = new Map(),
deselectedDataSeries: SeriesIdentifier[] = [],
customXDomain?: DomainRange | Domain,
orderOrdinalBinsBy?: OrderBy,
enableVislibSeriesSort?: boolean,
): SeriesDomainsAndData {
const { dataSeriesBySpecId, xValues, seriesCollection, fallbackScale } = getDataSeriesBySpecId(
seriesSpecs,
deselectedDataSeries,
orderOrdinalBinsBy,
enableVislibSeriesSort,
);

// compute the x domain merging any custom domain
const specsArray = [...seriesSpecs.values()];
const xDomain = mergeXDomain(specsArray, xValues, customXDomain, fallbackScale);
Expand Down
Loading

0 comments on commit 45ea0c6

Please sign in to comment.