From 987eff82a35cafd979d2a3c636feddbf9d851d1a Mon Sep 17 00:00:00 2001 From: Ryan McKinley Date: Tue, 18 Apr 2023 14:07:27 -0700 Subject: [PATCH] FieldValues: Implement array accessors for deprecated Vector types (#66807) --- .betterer.results | 26 ++++++++++--- .../transformers/calculateField.ts | 38 +++++++++++-------- .../transformers/convertFieldType.ts | 2 +- packages/grafana-data/src/types/vector.ts | 27 +++++++++++++ .../src/vector/AppendedVectors.test.ts | 3 ++ .../src/vector/AppendedVectors.ts | 5 ++- .../grafana-data/src/vector/AsNumberVector.ts | 17 ++------- .../src/vector/BinaryOperationVector.test.ts | 8 +++- .../src/vector/BinaryOperationVector.ts | 29 ++++---------- .../src/vector/CircularVector.test.ts | 4 ++ .../grafana-data/src/vector/CircularVector.ts | 3 ++ .../src/vector/ConstantVector.test.ts | 3 +- .../grafana-data/src/vector/ConstantVector.ts | 26 ++----------- .../src/vector/FormattedVector.ts | 18 ++------- .../grafana-data/src/vector/IndexVector.ts | 23 +++++------ packages/grafana-data/src/vector/RowVector.ts | 33 ---------------- .../src/vector/SortedVector.test.ts | 13 +++++++ .../grafana-data/src/vector/SortedVector.ts | 3 +- 18 files changed, 137 insertions(+), 144 deletions(-) delete mode 100644 packages/grafana-data/src/vector/RowVector.ts create mode 100644 packages/grafana-data/src/vector/SortedVector.test.ts diff --git a/.betterer.results b/.betterer.results index cf5cc6f394d..d49d92f14c5 100644 --- a/.betterer.results +++ b/.betterer.results @@ -270,8 +270,11 @@ exports[`better eslint`] = { [0, 0, 0, "Unexpected any. Specify a different type.", "0"] ], "packages/grafana-data/src/transformations/transformers/calculateField.ts:5381": [ - [0, 0, 0, "Do not use any type assertions.", "0"], - [0, 0, 0, "Do not use any type assertions.", "1"] + [0, 0, 0, "Unexpected any. Specify a different type.", "0"], + [0, 0, 0, "Unexpected any. Specify a different type.", "1"], + [0, 0, 0, "Do not use any type assertions.", "2"], + [0, 0, 0, "Do not use any type assertions.", "3"], + [0, 0, 0, "Unexpected any. Specify a different type.", "4"] ], "packages/grafana-data/src/transformations/transformers/ensureColumns.ts:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"] @@ -497,7 +500,9 @@ exports[`better eslint`] = { [0, 0, 0, "Unexpected any. Specify a different type.", "8"], [0, 0, 0, "Unexpected any. Specify a different type.", "9"], [0, 0, 0, "Unexpected any. Specify a different type.", "10"], - [0, 0, 0, "Unexpected any. Specify a different type.", "11"] + [0, 0, 0, "Unexpected any. Specify a different type.", "11"], + [0, 0, 0, "Do not use any type assertions.", "12"], + [0, 0, 0, "Unexpected any. Specify a different type.", "13"] ], "packages/grafana-data/src/utils/OptionsUIBuilders.ts:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"], @@ -664,15 +669,23 @@ exports[`better eslint`] = { [0, 0, 0, "Unexpected any. Specify a different type.", "1"], [0, 0, 0, "Unexpected any. Specify a different type.", "2"] ], + "packages/grafana-data/src/vector/AsNumberVector.ts:5381": [ + [0, 0, 0, "Do not use any type assertions.", "0"] + ], + "packages/grafana-data/src/vector/BinaryOperationVector.ts:5381": [ + [0, 0, 0, "Do not use any type assertions.", "0"] + ], "packages/grafana-data/src/vector/CircularVector.ts:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"], [0, 0, 0, "Unexpected any. Specify a different type.", "1"] ], "packages/grafana-data/src/vector/ConstantVector.ts:5381": [ - [0, 0, 0, "Unexpected any. Specify a different type.", "0"] + [0, 0, 0, "Unexpected any. Specify a different type.", "0"], + [0, 0, 0, "Do not use any type assertions.", "1"] ], "packages/grafana-data/src/vector/FormattedVector.ts:5381": [ - [0, 0, 0, "Unexpected any. Specify a different type.", "0"] + [0, 0, 0, "Unexpected any. Specify a different type.", "0"], + [0, 0, 0, "Do not use any type assertions.", "1"] ], "packages/grafana-data/src/vector/FunctionalVector.ts:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"], @@ -687,6 +700,9 @@ exports[`better eslint`] = { [0, 0, 0, "Unexpected any. Specify a different type.", "9"], [0, 0, 0, "Unexpected any. Specify a different type.", "10"] ], + "packages/grafana-data/src/vector/IndexVector.ts:5381": [ + [0, 0, 0, "Do not use any type assertions.", "0"] + ], "packages/grafana-data/src/vector/SortedVector.ts:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"] ], diff --git a/packages/grafana-data/src/transformations/transformers/calculateField.ts b/packages/grafana-data/src/transformations/transformers/calculateField.ts index 76d5b515e72..3a5a9218ffc 100644 --- a/packages/grafana-data/src/transformations/transformers/calculateField.ts +++ b/packages/grafana-data/src/transformations/transformers/calculateField.ts @@ -3,11 +3,8 @@ import { map } from 'rxjs/operators'; import { getTimeField } from '../../dataframe/processDataFrame'; import { getFieldDisplayName } from '../../field'; -import { DataFrame, DataTransformerInfo, Field, FieldType, NullValueMode, Vector } from '../../types'; +import { DataFrame, DataTransformerInfo, Field, FieldType, NullValueMode } from '../../types'; import { BinaryOperationID, binaryOperators } from '../../utils/binaryOperators'; -import { BinaryOperationVector, ConstantVector } from '../../vector'; -import { AsNumberVector } from '../../vector/AsNumberVector'; -import { RowVector } from '../../vector/RowVector'; import { doStandardCalcs, fieldReducers, ReducerID } from '../fieldReducer'; import { getFieldMatcher } from '../matchers'; import { FieldMatcherID } from '../matchers/ids'; @@ -61,7 +58,7 @@ export interface CalculateFieldTransformerOptions { // TODO: config?: FieldConfig; or maybe field overrides? since the UI exists } -type ValuesCreator = (data: DataFrame) => Vector; +type ValuesCreator = (data: DataFrame) => any[]; export const calculateFieldTransformer: DataTransformerInfo = { id: DataTransformerID.calculateField, @@ -181,7 +178,7 @@ function getReduceRowCreator(options: ReduceOptions, allFrames: DataFrame[]): Va return (frame: DataFrame) => { // Find the columns that should be examined - const columns: Vector[] = []; + const columns: any[] = []; for (const field of frame.fields) { if (matcher(field, frame, allFrames)) { columns.push(field.values); @@ -189,26 +186,31 @@ function getReduceRowCreator(options: ReduceOptions, allFrames: DataFrame[]): Va } // Prepare a "fake" field for the row - const iter = new RowVector(columns); + const size = columns.length; const row: Field = { name: 'temp', - values: iter, + values: new Array(size), type: FieldType.number, config: {}, }; const vals: number[] = []; for (let i = 0; i < frame.length; i++) { - iter.rowIndex = i; - const val = reducer(row, ignoreNulls, nullAsZero)[options.reducer]; - vals.push(val); + for (let j = 0; j < size; j++) { + row.values[j] = columns[j][i]; + } + vals.push(reducer(row, ignoreNulls, nullAsZero)[options.reducer]); } return vals; }; } -function findFieldValuesWithNameOrConstant(frame: DataFrame, name: string, allFrames: DataFrame[]): Vector | undefined { +function findFieldValuesWithNameOrConstant( + frame: DataFrame, + name: string, + allFrames: DataFrame[] +): number[] | undefined { if (!name) { return undefined; } @@ -216,7 +218,7 @@ function findFieldValuesWithNameOrConstant(frame: DataFrame, name: string, allFr for (const f of frame.fields) { if (name === getFieldDisplayName(f, frame, allFrames)) { if (f.type === FieldType.boolean) { - return new AsNumberVector(f.values); + return f.values.map((v) => (v ? 1 : 0)); } return f.values; } @@ -224,7 +226,7 @@ function findFieldValuesWithNameOrConstant(frame: DataFrame, name: string, allFr const v = parseFloat(name); if (!isNaN(v)) { - return new ConstantVector(v, frame.length); + return new Array(frame.length).fill(v); } return undefined; @@ -237,10 +239,14 @@ function getBinaryCreator(options: BinaryOptions, allFrames: DataFrame[]): Value const left = findFieldValuesWithNameOrConstant(frame, options.left, allFrames); const right = findFieldValuesWithNameOrConstant(frame, options.right, allFrames); if (!left || !right || !operator) { - return undefined as unknown as Vector; + return undefined as unknown as any[]; } - return new BinaryOperationVector(left, right, operator.operation); + const arr = new Array(left.length); + for (let i = 0; i < arr.length; i++) { + arr[i] = operator.operation(left[i], right[i]); + } + return arr; }; } diff --git a/packages/grafana-data/src/transformations/transformers/convertFieldType.ts b/packages/grafana-data/src/transformations/transformers/convertFieldType.ts index ed2340de66a..07ca981597b 100644 --- a/packages/grafana-data/src/transformations/transformers/convertFieldType.ts +++ b/packages/grafana-data/src/transformations/transformers/convertFieldType.ts @@ -147,7 +147,7 @@ function fieldToNumberField(field: Field): Field { for (let n = 0; n < numValues.length; n++) { let toBeConverted = numValues[n]; - if (valuesAsStrings) { + if (valuesAsStrings && toBeConverted != null) { // some numbers returned from datasources have commas // strip the commas, coerce the string to a number toBeConverted = toBeConverted.replace(/,/g, ''); diff --git a/packages/grafana-data/src/types/vector.ts b/packages/grafana-data/src/types/vector.ts index 29e7fc04d51..b63c7542490 100644 --- a/packages/grafana-data/src/types/vector.ts +++ b/packages/grafana-data/src/types/vector.ts @@ -97,3 +97,30 @@ export interface ReadWriteVector extends Vector {} * @deprecated -- this is now part of the base Vector interface */ export interface MutableVector extends ReadWriteVector {} + +/** + * This is an extremely inefficient Vector wrapper that allows vectors to + * be treated as arrays. We should avoid using this wrapper, but it is helpful + * for a clean migration to arrays + * + * @deprecated + */ +export function makeArrayIndexableVector(v: T): T { + return new Proxy(v, { + get(target: Vector, property: string, receiver: Vector) { + const idx = +property; + if (String(idx) === property) { + return target.get(idx); + } + return Reflect.get(target, property, receiver); + }, + set(target: Vector, property: string, value: any, receiver: Vector) { + const idx = +property; + if (String(idx) === property) { + target.set(idx, value); + return true; + } + return Reflect.set(target, property, value, receiver); + }, + }) as T; +} diff --git a/packages/grafana-data/src/vector/AppendedVectors.test.ts b/packages/grafana-data/src/vector/AppendedVectors.test.ts index 00c1f4272b2..86e54546785 100644 --- a/packages/grafana-data/src/vector/AppendedVectors.test.ts +++ b/packages/grafana-data/src/vector/AppendedVectors.test.ts @@ -8,6 +8,9 @@ describe('Check Appending Vector', () => { appended.append(new ArrayVector([4, 5, 6])); appended.append(new ArrayVector([7, 8, 9])); expect(appended.length).toEqual(9); + expect(appended[0]).toEqual(1); + expect(appended[1]).toEqual(2); + expect(appended[100]).toEqual(undefined); appended.setLength(5); expect(appended.length).toEqual(5); diff --git a/packages/grafana-data/src/vector/AppendedVectors.ts b/packages/grafana-data/src/vector/AppendedVectors.ts index 805b88fc273..af9873a69c2 100644 --- a/packages/grafana-data/src/vector/AppendedVectors.ts +++ b/packages/grafana-data/src/vector/AppendedVectors.ts @@ -1,4 +1,4 @@ -import { Vector } from '../types/vector'; +import { Vector, makeArrayIndexableVector } from '../types/vector'; import { FunctionalVector } from './FunctionalVector'; import { vectorToArray } from './vectorToArray'; @@ -14,7 +14,7 @@ interface AppendedVectorInfo { * RAM -- rather than allocate a new array the size of all previous arrays, this just * points the correct index to their original array values * - * @deprecated use a simple Arrays + * @deprecated use a simple Arrays. NOTE this is not used in grafana core */ export class AppendedVectors extends FunctionalVector { length = 0; @@ -23,6 +23,7 @@ export class AppendedVectors extends FunctionalVector { constructor(startAt = 0) { super(); this.length = startAt; + return makeArrayIndexableVector(this); } /** diff --git a/packages/grafana-data/src/vector/AsNumberVector.ts b/packages/grafana-data/src/vector/AsNumberVector.ts index c51d77179c6..ac8644442be 100644 --- a/packages/grafana-data/src/vector/AsNumberVector.ts +++ b/packages/grafana-data/src/vector/AsNumberVector.ts @@ -1,23 +1,14 @@ import { Vector } from '../types'; -import { FunctionalVector } from './FunctionalVector'; - /** * This will force all values to be numbers * * @public - * @deprecated use a simple Arrays + * @deprecated use a simple Arrays. NOTE: Not used in grafana core */ -export class AsNumberVector extends FunctionalVector { - constructor(private field: Vector) { +export class AsNumberVector extends Array { + constructor(field: Vector) { super(); - } - - get length() { - return this.field.length; - } - - get(index: number) { - return +this.field.get(index); + return field.map((v) => +v) as AsNumberVector; } } diff --git a/packages/grafana-data/src/vector/BinaryOperationVector.test.ts b/packages/grafana-data/src/vector/BinaryOperationVector.test.ts index d26ed06d3bb..e59ba476e33 100644 --- a/packages/grafana-data/src/vector/BinaryOperationVector.test.ts +++ b/packages/grafana-data/src/vector/BinaryOperationVector.test.ts @@ -11,9 +11,13 @@ describe('ScaledVector', () => { const operation = binaryOperators.get(BinaryOperationID.Multiply).operation; const v = new BinaryOperationVector(source, new ConstantVector(scale, source.length), operation); expect(v.length).toEqual(source.length); - // expect(v.push(10)).toEqual(source.length); // not implemented - for (let i = 0; i < 10; i++) { + // Accessed with getters + for (let i = 0; i < 4; i++) { expect(v.get(i)).toEqual(source.get(i) * scale); } + // Accessed with array index + for (let i = 0; i < 4; i++) { + expect(v[i]).toEqual(source[i] * scale); + } }); }); diff --git a/packages/grafana-data/src/vector/BinaryOperationVector.ts b/packages/grafana-data/src/vector/BinaryOperationVector.ts index 74453fd6f75..933d3dafe8a 100644 --- a/packages/grafana-data/src/vector/BinaryOperationVector.ts +++ b/packages/grafana-data/src/vector/BinaryOperationVector.ts @@ -1,31 +1,18 @@ import { Vector } from '../types/vector'; import { BinaryOperation } from '../utils/binaryOperators'; -import { FunctionalVector } from './FunctionalVector'; -import { vectorToArray } from './vectorToArray'; - /** * @public - * @deprecated use a simple Arrays + * @deprecated use a simple Arrays. NOTE: Not used in grafana core */ -export class BinaryOperationVector extends FunctionalVector { - constructor(private left: Vector, private right: Vector, private operation: BinaryOperation) { +export class BinaryOperationVector extends Array { + constructor(left: Vector, right: Vector, operation: BinaryOperation) { super(); - } - get length(): number { - return this.left.length; - } - - get(index: number): number { - return this.operation(this.left.get(index), this.right.get(index)); - } - - toArray(): number[] { - return vectorToArray(this); - } - - toJSON(): number[] { - return vectorToArray(this); + const arr = new Array(left.length); + for (let i = 0; i < arr.length; i++) { + arr[i] = operation(left[i], right[i]); + } + return arr as BinaryOperationVector; } } diff --git a/packages/grafana-data/src/vector/CircularVector.test.ts b/packages/grafana-data/src/vector/CircularVector.test.ts index 3ed934e89bc..0c582e8e155 100644 --- a/packages/grafana-data/src/vector/CircularVector.test.ts +++ b/packages/grafana-data/src/vector/CircularVector.test.ts @@ -5,6 +5,10 @@ describe('Check Circular Vector', () => { const buffer = [1, 2, 3]; const v = new CircularVector({ buffer }); // tail is default option expect(v.toArray()).toEqual([1, 2, 3]); + expect(v[0]).toEqual(1); + expect(v[1]).toEqual(2); + expect(v[2]).toEqual(3); + expect(v[3]).toEqual(1); // loops back to one v.add(4); expect(v.toArray()).toEqual([2, 3, 4]); diff --git a/packages/grafana-data/src/vector/CircularVector.ts b/packages/grafana-data/src/vector/CircularVector.ts index e3594b25174..aa70ae27578 100644 --- a/packages/grafana-data/src/vector/CircularVector.ts +++ b/packages/grafana-data/src/vector/CircularVector.ts @@ -1,3 +1,5 @@ +import { makeArrayIndexableVector } from '../types'; + import { FunctionalVector } from './FunctionalVector'; interface CircularOptions { @@ -34,6 +36,7 @@ export class CircularVector extends FunctionalVector { if (options.capacity) { this.setCapacity(options.capacity); } + return makeArrayIndexableVector(this); } /** diff --git a/packages/grafana-data/src/vector/ConstantVector.test.ts b/packages/grafana-data/src/vector/ConstantVector.test.ts index b1b942209ee..2d94fc34428 100644 --- a/packages/grafana-data/src/vector/ConstantVector.test.ts +++ b/packages/grafana-data/src/vector/ConstantVector.test.ts @@ -10,8 +10,9 @@ describe('ConstantVector', () => { expect(v.get(1)).toEqual(value); // Now check all of them - for (let i = 0; i < 10; i++) { + for (let i = 0; i < 7; i++) { expect(v.get(i)).toEqual(value); + expect(v[i]).toEqual(value); } }); }); diff --git a/packages/grafana-data/src/vector/ConstantVector.ts b/packages/grafana-data/src/vector/ConstantVector.ts index 5eec8f31abf..65a96ec4143 100644 --- a/packages/grafana-data/src/vector/ConstantVector.ts +++ b/packages/grafana-data/src/vector/ConstantVector.ts @@ -1,28 +1,10 @@ -import { FunctionalVector } from './FunctionalVector'; - /** * @public - * @deprecated use a simple Arrays + * @deprecated use a simple Arrays. NOTE: Not used in grafana core. */ -export class ConstantVector extends FunctionalVector { - constructor(private value: T, private len: number) { +export class ConstantVector extends Array { + constructor(value: T, len: number) { super(); - } - - get length() { - return this.len; - } - - get(index: number): T { - return this.value; - } - - toArray(): T[] { - const arr = new Array(this.length); - return arr.fill(this.value); - } - - toJSON(): T[] { - return this.toArray(); + return new Array(len).fill(value) as ConstantVector; } } diff --git a/packages/grafana-data/src/vector/FormattedVector.ts b/packages/grafana-data/src/vector/FormattedVector.ts index a6378221d4f..6dfbf087f6d 100644 --- a/packages/grafana-data/src/vector/FormattedVector.ts +++ b/packages/grafana-data/src/vector/FormattedVector.ts @@ -2,23 +2,13 @@ import { DisplayProcessor } from '../types'; import { Vector } from '../types/vector'; import { formattedValueToString } from '../valueFormats'; -import { FunctionalVector } from './FunctionalVector'; - /** * @public - * @deprecated use a simple Arrays + * @deprecated use a simple Arrays. NOTE: not used in grafana core. */ -export class FormattedVector extends FunctionalVector { - constructor(private source: Vector, private formatter: DisplayProcessor) { +export class FormattedVector extends Array { + constructor(source: Vector, formatter: DisplayProcessor) { super(); - } - - get length() { - return this.source.length; - } - - get(index: number): string { - const v = this.source.get(index); - return formattedValueToString(this.formatter(v)); + return source.map((v) => formattedValueToString(formatter(v))) as FormattedVector; } } diff --git a/packages/grafana-data/src/vector/IndexVector.ts b/packages/grafana-data/src/vector/IndexVector.ts index f69fb51dc5d..c32eb98ca60 100644 --- a/packages/grafana-data/src/vector/IndexVector.ts +++ b/packages/grafana-data/src/vector/IndexVector.ts @@ -1,29 +1,26 @@ import { Field, FieldType } from '../types'; -import { FunctionalVector } from './FunctionalVector'; - /** * IndexVector is a simple vector implementation that returns the index value * for each element in the vector. It is functionally equivolant a vector backed * by an array with values: `[0,1,2,...,length-1]` * - * @deprecated use a simple Arrays + * @deprecated use a simple Arrays. NOTE: not used in grafana core */ -export class IndexVector extends FunctionalVector { - constructor(private len: number) { +export class IndexVector extends Array { + constructor(len: number) { super(); - } - - get length() { - return this.len; - } - - get(index: number): number { - return index; + const arr = new Array(len); + for (let i = 0; i < len; i++) { + arr[i] = i; + } + return arr as IndexVector; } /** * Returns a field representing the range [0 ... length-1] + * + * @deprecated */ static newField(len: number): Field { return { diff --git a/packages/grafana-data/src/vector/RowVector.ts b/packages/grafana-data/src/vector/RowVector.ts deleted file mode 100644 index 793c13c0df6..00000000000 --- a/packages/grafana-data/src/vector/RowVector.ts +++ /dev/null @@ -1,33 +0,0 @@ -import { Vector } from '../types'; - -import { FunctionalVector } from './FunctionalVector'; -import { vectorToArray } from './vectorToArray'; - -/** - * RowVector makes the row values look like a vector - * @internal - * @deprecated use a simple Arrays - */ -export class RowVector extends FunctionalVector { - constructor(private columns: Vector[]) { - super(); - } - - rowIndex = 0; - - get length(): number { - return this.columns.length; - } - - get(index: number): number { - return this.columns[index].get(this.rowIndex); - } - - toArray(): number[] { - return vectorToArray(this); - } - - toJSON(): number[] { - return vectorToArray(this); - } -} diff --git a/packages/grafana-data/src/vector/SortedVector.test.ts b/packages/grafana-data/src/vector/SortedVector.test.ts new file mode 100644 index 00000000000..c8418e87298 --- /dev/null +++ b/packages/grafana-data/src/vector/SortedVector.test.ts @@ -0,0 +1,13 @@ +import { ArrayVector } from './ArrayVector'; +import { SortedVector } from './SortedVector'; + +describe('SortedVector', () => { + it('Should support sorting', () => { + const values = new ArrayVector([1, 5, 2, 4]); + const sorted = new SortedVector(values, [0, 2, 3, 1]); + expect(sorted.toArray()).toEqual([1, 2, 4, 5]); + + // The proxy should still be an instance of SortedVector (used in timeseries) + expect(sorted instanceof SortedVector).toBeTruthy(); + }); +}); diff --git a/packages/grafana-data/src/vector/SortedVector.ts b/packages/grafana-data/src/vector/SortedVector.ts index 780005c0000..5e3dd5176ba 100644 --- a/packages/grafana-data/src/vector/SortedVector.ts +++ b/packages/grafana-data/src/vector/SortedVector.ts @@ -1,4 +1,4 @@ -import { Vector } from '../types/vector'; +import { makeArrayIndexableVector, Vector } from '../types/vector'; import { FunctionalVector } from './FunctionalVector'; import { vectorToArray } from './vectorToArray'; @@ -11,6 +11,7 @@ import { vectorToArray } from './vectorToArray'; export class SortedVector extends FunctionalVector { constructor(private source: Vector, private order: number[]) { super(); + return makeArrayIndexableVector(this); } get length(): number {