Skip to content

Commit

Permalink
fix(plugin-chart-echarts): support truncated numeric x-axis (#26215)
Browse files Browse the repository at this point in the history
Co-authored-by: Michael S. Molina <michael.s.molina@gmail.com>
  • Loading branch information
villebro and michael-s-molina committed Dec 8, 2023
1 parent 8eb6bbb commit b0905ce
Show file tree
Hide file tree
Showing 18 changed files with 375 additions and 265 deletions.
2 changes: 1 addition & 1 deletion helm/superset/Chart.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
# limitations under the License.
#
apiVersion: v2
appVersion: "3.0.2"
appVersion: "3.0.3"
description: Apache Superset is a modern, enterprise-ready business intelligence web application
name: superset
icon: https://artifacthub.io/image/68c1d717-0e97-491f-b046-754e46f46922@2x
Expand Down
474 changes: 237 additions & 237 deletions helm/superset/README.md

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ import {
ForecastSeriesEnum,
Refs,
} from '../types';
import { parseYAxisBound } from '../utils/controls';
import { parseAxisBound } from '../utils/controls';
import {
getOverMaxHiddenFormatter,
dedupSeries,
Expand Down Expand Up @@ -343,9 +343,9 @@ export default function transformProps(
});

// yAxisBounds need to be parsed to replace incompatible values with undefined
let [min, max] = (yAxisBounds || []).map(parseYAxisBound);
let [min, max] = (yAxisBounds || []).map(parseAxisBound);
let [minSecondary, maxSecondary] = (yAxisBoundsSecondary || []).map(
parseYAxisBound,
parseAxisBound,
);

const array = ensureIsArray(chartProps.rawFormData?.time_compare);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ import {
richTooltipSection,
seriesOrderSection,
percentageThresholdControl,
truncateXAxis,
xAxisBounds,
} from '../../controls';
import { AreaChartStackControlOptions } from '../../constants';

Expand Down Expand Up @@ -241,6 +243,8 @@ const config: ControlPanelConfig = {
},
},
],
[truncateXAxis],
[xAxisBounds],
[
{
name: 'truncateYAxis',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ import {
richTooltipSection,
seriesOrderSection,
showValueSection,
truncateXAxis,
xAxisBounds,
} from '../../../controls';

import { OrientationType } from '../../types';
Expand Down Expand Up @@ -224,6 +226,8 @@ function createAxisControl(axis: 'x' | 'y'): ControlSetRow[] {
},
},
],
[truncateXAxis],
[xAxisBounds],
[
{
name: 'truncateYAxis',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ import {
richTooltipSection,
seriesOrderSection,
showValueSection,
truncateXAxis,
xAxisBounds,
} from '../../../controls';

const {
Expand Down Expand Up @@ -229,6 +231,8 @@ const config: ControlPanelConfig = {
},
},
],
[truncateXAxis],
[xAxisBounds],
[
{
name: 'truncateYAxis',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ import {
richTooltipSection,
seriesOrderSection,
showValueSection,
truncateXAxis,
xAxisBounds,
} from '../../../controls';

const {
Expand Down Expand Up @@ -173,6 +175,8 @@ const config: ControlPanelConfig = {
},
},
],
[truncateXAxis],
[xAxisBounds],
[
{
name: 'truncateYAxis',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ import {
richTooltipSection,
seriesOrderSection,
showValueSectionWithoutStack,
truncateXAxis,
xAxisBounds,
} from '../../../controls';

const {
Expand Down Expand Up @@ -173,6 +175,8 @@ const config: ControlPanelConfig = {
},
},
],
[truncateXAxis],
[xAxisBounds],
[
{
name: 'truncateYAxis',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ import {
richTooltipSection,
seriesOrderSection,
showValueSection,
truncateXAxis,
xAxisBounds,
} from '../../controls';

const {
Expand Down Expand Up @@ -223,6 +225,8 @@ const config: ControlPanelConfig = {
},
},
],
[truncateXAxis],
[xAxisBounds],
[
{
name: 'truncateYAxis',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export const DEFAULT_FORM_DATA: EchartsTimeseriesFormData = {
seriesType: EchartsTimeseriesSeriesType.Line,
stack: false,
tooltipTimeFormat: 'smart_date',
truncateXAxis: true,
truncateYAxis: false,
yAxisBounds: [null, null],
zoomable: false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ import {
} from './types';
import { DEFAULT_FORM_DATA } from './constants';
import { ForecastSeriesEnum, ForecastValue, Refs } from '../types';
import { parseYAxisBound } from '../utils/controls';
import { parseAxisBound } from '../utils/controls';
import {
calculateLowerLogTick,
dedupSeries,
Expand All @@ -64,6 +64,7 @@ import {
getAxisType,
getColtypesMapping,
getLegendProps,
getMinAndMaxFromBounds,
} from '../utils/series';
import {
extractAnnotationLabels,
Expand Down Expand Up @@ -159,8 +160,10 @@ export default function transformProps(
stack,
tooltipTimeFormat,
tooltipSortByMetric,
truncateXAxis,
truncateYAxis,
xAxis: xAxisOrig,
xAxisBounds,
xAxisLabelRotation,
xAxisSortSeries,
xAxisSortSeriesAscending,
Expand Down Expand Up @@ -386,15 +389,20 @@ export default function transformProps(
}
});

// yAxisBounds need to be parsed to replace incompatible values with undefined
let [min, max] = (yAxisBounds || []).map(parseYAxisBound);
// axis bounds need to be parsed to replace incompatible values with undefined
const [xAxisMin, xAxisMax] = (xAxisBounds || []).map(parseAxisBound);
let [yAxisMin, yAxisMax] = (yAxisBounds || []).map(parseAxisBound);

// default to 0-100% range when doing row-level contribution chart
if ((contributionMode === 'row' || isAreaExpand) && stack) {
if (min === undefined) min = 0;
if (max === undefined) max = 1;
} else if (logAxis && min === undefined && minPositiveValue !== undefined) {
min = calculateLowerLogTick(minPositiveValue);
if (yAxisMin === undefined) yAxisMin = 0;
if (yAxisMax === undefined) yAxisMax = 1;
} else if (
logAxis &&
yAxisMin === undefined &&
minPositiveValue !== undefined
) {
yAxisMin = calculateLowerLogTick(minPositiveValue);
}

const tooltipFormatter =
Expand Down Expand Up @@ -450,12 +458,14 @@ export default function transformProps(
xAxisType === AxisType.time && timeGrainSqla
? TIMEGRAIN_TO_TIMESTAMP[timeGrainSqla]
: 0,
...getMinAndMaxFromBounds(xAxisType, truncateXAxis, xAxisMin, xAxisMax),
};

let yAxis: any = {
...defaultYAxis,
type: logAxis ? AxisType.log : AxisType.value,
min,
max,
min: yAxisMin,
max: yAxisMax,
minorTick: { show: true },
minorSplitLine: { show: minorSplitLine },
axisLabel: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,12 @@ export type EchartsTimeseriesFormData = QueryFormData & {
stack: StackType;
timeCompare?: string[];
tooltipTimeFormat?: string;
truncateXAxis: boolean;
truncateYAxis: boolean;
yAxisFormat?: string;
xAxisTimeFormat?: string;
timeGrainSqla?: TimeGranularity;
xAxisBounds: [number | undefined | null, number | undefined | null];
yAxisBounds: [number | undefined | null, number | undefined | null];
zoomable: boolean;
richTooltip: boolean;
Expand Down
31 changes: 31 additions & 0 deletions superset-frontend/plugins/plugin-chart-echarts/src/controls.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -248,3 +248,34 @@ export const seriesOrderSection: ControlSetRow[] = [
[sortSeriesType],
[sortSeriesAscending],
];

export const truncateXAxis: ControlSetItem = {
name: 'truncateXAxis',
config: {
type: 'CheckboxControl',
label: t('Truncate X Axis'),
default: DEFAULT_FORM_DATA.truncateXAxis,
renderTrigger: true,
description: t(
'Truncate X Axis. Can be overridden by specifying a min or max bound. Only applicable for numercal X axis.',
),
},
};

export const xAxisBounds: ControlSetItem = {
name: 'xAxisBounds',
config: {
type: 'BoundsControl',
label: t('X Axis Bounds'),
renderTrigger: true,
default: DEFAULT_FORM_DATA.xAxisBounds,
description: t(
'Bounds for numerical X axis. Not applicable for temporal or categorical axes. ' +
'When left empty, the bounds are dynamically defined based on the min/max of the data. ' +
"Note that this feature will only expand the axis range. It won't " +
"narrow the data's extent.",
),
visibility: ({ controls }: ControlPanelsContainerProps) =>
Boolean(controls?.truncateXAxis?.value),
},
};
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import { validateNumber } from '@superset-ui/core';

// eslint-disable-next-line import/prefer-default-export
export function parseYAxisBound(
export function parseAxisBound(
bound?: string | number | null,
): number | undefined {
if (bound === undefined || bound === null || Number.isNaN(Number(bound))) {
Expand Down
14 changes: 14 additions & 0 deletions superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
Original file line number Diff line number Diff line change
Expand Up @@ -543,3 +543,17 @@ export function calculateLowerLogTick(minPositiveValue: number) {
const logBase10 = Math.floor(Math.log10(minPositiveValue));
return Math.pow(10, logBase10);
}

export function getMinAndMaxFromBounds(
axisType: AxisType,
truncateAxis: boolean,
min?: number,
max?: number,
): { min: number | 'dataMin'; max: number | 'dataMax' } | {} {
return truncateAxis && axisType === AxisType.value
? {
min: min === undefined ? 'dataMin' : min,
max: max === undefined ? 'dataMax' : max,
}
: {};
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,22 +16,22 @@
* specific language governing permissions and limitations
* under the License.
*/
import { parseYAxisBound } from '../../src/utils/controls';
import { parseAxisBound } from '../../src/utils/controls';

describe('parseYAxisBound', () => {
it('should return undefined for invalid values', () => {
expect(parseYAxisBound(null)).toBeUndefined();
expect(parseYAxisBound(undefined)).toBeUndefined();
expect(parseYAxisBound(NaN)).toBeUndefined();
expect(parseYAxisBound('abc')).toBeUndefined();
expect(parseAxisBound(null)).toBeUndefined();
expect(parseAxisBound(undefined)).toBeUndefined();
expect(parseAxisBound(NaN)).toBeUndefined();
expect(parseAxisBound('abc')).toBeUndefined();
});

it('should return numeric value for valid values', () => {
expect(parseYAxisBound(0)).toEqual(0);
expect(parseYAxisBound('0')).toEqual(0);
expect(parseYAxisBound(1)).toEqual(1);
expect(parseYAxisBound('1')).toEqual(1);
expect(parseYAxisBound(10.1)).toEqual(10.1);
expect(parseYAxisBound('10.1')).toEqual(10.1);
expect(parseAxisBound(0)).toEqual(0);
expect(parseAxisBound('0')).toEqual(0);
expect(parseAxisBound(1)).toEqual(1);
expect(parseAxisBound('1')).toEqual(1);
expect(parseAxisBound(10.1)).toEqual(10.1);
expect(parseAxisBound('10.1')).toEqual(10.1);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import {
getChartPadding,
getLegendProps,
getOverMaxHiddenFormatter,
getMinAndMaxFromBounds,
sanitizeHtml,
sortAndFilterSeries,
sortRows,
Expand Down Expand Up @@ -879,3 +880,30 @@ test('getAxisType', () => {
expect(getAxisType(GenericDataType.BOOLEAN)).toEqual(AxisType.category);
expect(getAxisType(GenericDataType.STRING)).toEqual(AxisType.category);
});

test('getMinAndMaxFromBounds returns empty object when not truncating', () => {
expect(getMinAndMaxFromBounds(AxisType.value, false, 10, 100)).toEqual({});
});

test('getMinAndMaxFromBounds returns automatic bounds when truncating', () => {
expect(
getMinAndMaxFromBounds(AxisType.value, true, undefined, undefined),
).toEqual({
min: 'dataMin',
max: 'dataMax',
});
});

test('getMinAndMaxFromBounds returns automatic upper bound when truncating', () => {
expect(getMinAndMaxFromBounds(AxisType.value, true, 10, undefined)).toEqual({
min: 10,
max: 'dataMax',
});
});

test('getMinAndMaxFromBounds returns automatic lower bound when truncating', () => {
expect(getMinAndMaxFromBounds(AxisType.value, true, undefined, 100)).toEqual({
min: 'dataMin',
max: 100,
});
});
6 changes: 3 additions & 3 deletions superset/dashboards/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ class DashboardGetResponseSchema(Schema):
changed_on_humanized = fields.String(data_key="changed_on_delta_humanized")
is_managed_externally = fields.Boolean(allow_none=True, dump_default=False)

# pylint: disable=unused-argument
# pylint: disable=unused-argument, no-self-use
@post_dump()
def post_dump(self, serialized: dict[str, Any], **kwargs: Any) -> dict[str, Any]:
if security_manager.is_guest_user():
Expand Down Expand Up @@ -260,7 +260,7 @@ class DashboardDatasetSchema(Schema):
granularity_sqla = fields.List(fields.List(fields.Str()))
normalize_columns = fields.Bool()

# pylint: disable=unused-argument
# pylint: disable=unused-argument, no-self-use
@post_dump()
def post_dump(self, serialized: dict[str, Any], **kwargs: Any) -> dict[str, Any]:
if security_manager.is_guest_user():
Expand All @@ -270,7 +270,7 @@ def post_dump(self, serialized: dict[str, Any], **kwargs: Any) -> dict[str, Any]


class BaseDashboardSchema(Schema):
# pylint: disable=no-self-use,unused-argument
# pylint: disable=no-self-use, unused-argument
@post_load
def post_load(self, data: dict[str, Any], **kwargs: Any) -> dict[str, Any]:
if data.get("slug"):
Expand Down

0 comments on commit b0905ce

Please sign in to comment.