From 4bd809b409a45589fabb2b85760f6e2fc6ea0cc5 Mon Sep 17 00:00:00 2001 From: Jesse Yang Date: Tue, 8 Mar 2022 16:01:30 -0800 Subject: [PATCH] Add sortNumericValues with different nan treatment --- .../src/utils/sortNumericValues.test.ts | 61 ++++++++++++++++++ .../src/utils/sortNumericValues.ts | 52 +++++++++++++++ .../TimeTable/SortNumberWithMixedTypes.ts | 36 ----------- .../visualizations/TimeTable/TimeTable.jsx | 11 +++- .../src/visualizations/TimeTable/sort.test.ts | 63 ------------------- 5 files changed, 123 insertions(+), 100 deletions(-) create mode 100644 superset-frontend/src/utils/sortNumericValues.test.ts create mode 100644 superset-frontend/src/utils/sortNumericValues.ts delete mode 100644 superset-frontend/src/visualizations/TimeTable/SortNumberWithMixedTypes.ts delete mode 100644 superset-frontend/src/visualizations/TimeTable/sort.test.ts diff --git a/superset-frontend/src/utils/sortNumericValues.test.ts b/superset-frontend/src/utils/sortNumericValues.test.ts new file mode 100644 index 0000000000000..5582afe22b5ad --- /dev/null +++ b/superset-frontend/src/utils/sortNumericValues.test.ts @@ -0,0 +1,61 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import sortNumericValues from './sortNumericValues'; + +test('should always sort null and NaNs to bottom', () => { + expect([null, 1, 2, '1', '5', NaN].sort(sortNumericValues)).toEqual([ + 1, + '1', + 2, + '5', + NaN, + null, + ]); + expect( + [null, 1, 2, '1', '5', NaN].sort((a, b) => + sortNumericValues(a, b, { descending: true }), + ), + ).toEqual(['5', 2, 1, '1', NaN, null]); +}); + +test('should treat null and NaN as smallest numbers', () => { + expect( + [null, 1, 2, '1', '5', NaN].sort((a, b) => + sortNumericValues(a, b, { nanTreatment: 'asSmallest' }), + ), + ).toEqual([null, NaN, 1, '1', 2, '5']); + expect( + [null, 1, 2, '1', '5', NaN].sort((a, b) => + sortNumericValues(a, b, { nanTreatment: 'asSmallest', descending: true }), + ), + ).toEqual(['5', 2, 1, '1', NaN, null]); +}); + +test('should treat null and NaN as largest numbers', () => { + expect( + [null, 1, 2, '1', '5', NaN].sort((a, b) => + sortNumericValues(a, b, { nanTreatment: 'asLargest' }), + ), + ).toEqual([1, '1', 2, '5', NaN, null]); + expect( + [null, 1, 2, '1', '5', NaN].sort((a, b) => + sortNumericValues(a, b, { nanTreatment: 'asLargest', descending: true }), + ), + ).toEqual([null, NaN, '5', 2, 1, '1']); +}); diff --git a/superset-frontend/src/utils/sortNumericValues.ts b/superset-frontend/src/utils/sortNumericValues.ts new file mode 100644 index 0000000000000..a1cc404f72971 --- /dev/null +++ b/superset-frontend/src/utils/sortNumericValues.ts @@ -0,0 +1,52 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { JsonPrimitive } from '@superset-ui/core'; + +export type NaNTreatment = 'alwaysLast' | 'asSmallest' | 'asLargest'; + +/** + * Array.sort(...) comparator for potential numeric values with the ability to + * treat null and NaN as the smallest or largest values or always sort to bottom. + */ +export default function sortNumericValues( + valueA: JsonPrimitive, + valueB: JsonPrimitive, + { + descending = false, + nanTreatment = 'alwaysLast', + }: { descending?: boolean; nanTreatment?: NaNTreatment } = {}, +) { + let orderByIsNaN = + Number(valueA == null) - Number(valueB == null) || + Number(Number.isNaN(Number(valueA))) - Number(Number.isNaN(Number(valueB))); + + // if A is null or NaN and B is not, `orderByIsNaN` is 1, + // which will make A come after B in the sorted array, + // since we want to treat A as smallest number, we need to flip the sign + // when sorting in ascending order. + if (nanTreatment === 'asSmallest' && !descending) { + orderByIsNaN = -orderByIsNaN; + } + if (nanTreatment === 'asLargest' && descending) { + orderByIsNaN = -orderByIsNaN; + } + return ( + orderByIsNaN || (Number(valueA) - Number(valueB)) * (descending ? -1 : 1) + ); +} diff --git a/superset-frontend/src/visualizations/TimeTable/SortNumberWithMixedTypes.ts b/superset-frontend/src/visualizations/TimeTable/SortNumberWithMixedTypes.ts deleted file mode 100644 index c3d212909d4f1..0000000000000 --- a/superset-frontend/src/visualizations/TimeTable/SortNumberWithMixedTypes.ts +++ /dev/null @@ -1,36 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -import { Row } from 'react-table'; - -const sortNumberWithMixedTypes = (rowA: Row, rowB: Row, columnId: string) => { - const rowAVal = rowA.values[columnId].props['data-value']; - const rowBVal = rowB.values[columnId].props['data-value']; - if (typeof rowAVal === 'number' && typeof rowBVal === 'number') { - return rowAVal - rowBVal; - } - if (typeof rowAVal === 'number') { - return 1; - } - if (typeof rowBVal === 'number') { - return -1; - } - return 0; -}; - -export default sortNumberWithMixedTypes; diff --git a/superset-frontend/src/visualizations/TimeTable/TimeTable.jsx b/superset-frontend/src/visualizations/TimeTable/TimeTable.jsx index 9f03e17ae7211..d12022f74c60e 100644 --- a/superset-frontend/src/visualizations/TimeTable/TimeTable.jsx +++ b/superset-frontend/src/visualizations/TimeTable/TimeTable.jsx @@ -27,14 +27,23 @@ import { MetricOption, } from '@superset-ui/chart-controls'; import moment from 'moment'; +import sortNumericValues from 'src/utils/sortNumericValues'; import FormattedNumber from './FormattedNumber'; import SparklineCell from './SparklineCell'; import './TimeTable.less'; -import sortNumberWithMixedTypes from './SortNumberWithMixedTypes'; const ACCESSIBLE_COLOR_BOUNDS = ['#ca0020', '#0571b0']; +const sortNumberWithMixedTypes = (rowA, rowB, columnId, descending) => + sortNumericValues( + rowA.values[columnId].props['data-value'], + rowB.values[columnId].props['data-value'], + { descending, nanTreatment: 'asSmallest' }, + ) * + // react-table sort function always expects -1 for smaller number + (descending ? -1 : 1); + function colorFromBounds(value, bounds, colorBounds = ACCESSIBLE_COLOR_BOUNDS) { if (bounds) { const [min, max] = bounds; diff --git a/superset-frontend/src/visualizations/TimeTable/sort.test.ts b/superset-frontend/src/visualizations/TimeTable/sort.test.ts deleted file mode 100644 index 384c9c6a9cdbc..0000000000000 --- a/superset-frontend/src/visualizations/TimeTable/sort.test.ts +++ /dev/null @@ -1,63 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -import { Row } from 'react-table'; -import sortFn from './SortNumberWithMixedTypes'; - -describe('sort Number and mixed types', () => { - const columnId = 'mLnVxkc1g'; - const rowA = { - values: { - metric: 'Albania', - mLnVxkc1g: { - props: { - 'data-value': null, - }, - }, - }, - } as unknown as Row; - const rowB = { - values: { - metric: 'Afghanistan', - mLnVxkc1g: { - props: { - 'data-value': -0.6749999999999972, - }, - }, - }, - } as unknown as Row; - const rowC = { - values: { - metric: 'Malawi', - mLnVxkc1g: { - props: { - 'data-value': 4.852999999999994, - }, - }, - }, - } as unknown as Row; - - it('should treat null values as smallest', () => { - expect(sortFn(rowA, rowB, columnId)).toBe(-1); - expect(sortFn(rowA, rowC, columnId)).toBe(-1); - expect(sortFn(rowB, rowC, columnId)).toBe( - rowB.values[columnId].props['data-value'] - - rowC.values[columnId].props['data-value'], - ); - }); -});