Skip to content

Commit

Permalink
GH-39131: [JS] Add at() for array like types (#40730)
Browse files Browse the repository at this point in the history
Simpler version of #40712 that
preserves `get`.
* GitHub Issue: #39131
  • Loading branch information
domoritz authored and raulcd committed Apr 29, 2024
1 parent bb9985f commit 3d690b7
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 26 deletions.
17 changes: 13 additions & 4 deletions js/src/recordbatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { Table } from './table.js';
import { Vector } from './vector.js';
import { Schema, Field } from './schema.js';
import { DataType, Struct, Null, TypeMap } from './type.js';
import { wrapIndex } from './util/vector.js';

import { instance as getVisitor } from './visitor/get.js';
import { instance as setVisitor } from './visitor/set.js';
Expand Down Expand Up @@ -116,7 +117,7 @@ export class RecordBatch<T extends TypeMap = any> {
}

/**
* Check whether an element is null.
* Check whether an row is null.
* @param index The index at which to read the validity bitmap.
*/
public isValid(index: number) {
Expand All @@ -125,15 +126,23 @@ export class RecordBatch<T extends TypeMap = any> {

/**
* Get a row by position.
* @param index The index of the element to read.
* @param index The index of the row to read.
*/
public get(index: number) {
return getVisitor.visit(this.data, index);
}

/**
* Get a row value by position.
* @param index The index of the row to read. A negative index will count back from the last row.
*/
public at(index: number) {
return this.get(wrapIndex(index, this.numRows));
}

/**
* Set a row by position.
* @param index The index of the element to write.
* @param index The index of the row to write.
* @param value The value to set.
*/
public set(index: number, value: Struct<T>['TValue']) {
Expand Down Expand Up @@ -175,7 +184,7 @@ export class RecordBatch<T extends TypeMap = any> {
/**
* Return a zero-copy sub-section of this RecordBatch.
* @param start The beginning of the specified portion of the RecordBatch.
* @param end The end of the specified portion of the RecordBatch. This is exclusive of the element at the index 'end'.
* @param end The end of the specified portion of the RecordBatch. This is exclusive of the row at the index 'end'.
*/
public slice(begin?: number, end?: number): RecordBatch<T> {
const [slice] = new Vector([this.data]).slice(begin, end).data;
Expand Down
11 changes: 10 additions & 1 deletion js/src/table.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ import { instance as indexOfVisitor } from './visitor/indexof.js';
import { instance as iteratorVisitor } from './visitor/iterator.js';

import { DataProps } from './data.js';
import { clampRange } from './util/vector.js';
import { clampRange, wrapIndex } from './util/vector.js';
import { ArrayDataType, BigIntArray, TypedArray, TypedArrayDataType } from './interfaces.js';
import { RecordBatch, _InternalEmptyPlaceholderRecordBatch } from './recordbatch.js';

Expand Down Expand Up @@ -196,6 +196,15 @@ export class Table<T extends TypeMap = any> {
// @ts-ignore
public get(index: number): Struct<T>['TValue'] | null { return null; }

/**
* Get an element value by position.
* @param index The index of the element to read. A negative index will count back from the last element.
*/
// @ts-ignore
public at(index: number): Struct<T>['TValue'] | null {
return this.get(wrapIndex(index, this.numRows));
}

/**
* Set an element value by position.
*
Expand Down
14 changes: 3 additions & 11 deletions js/src/util/vector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,8 @@ import { compareArrayLike } from '../util/buffer.js';
/** @ignore */
type RangeLike = { length: number; stride?: number };
/** @ignore */
type ClampThen<T extends RangeLike> = (source: T, index: number) => any;
/** @ignore */
type ClampRangeThen<T extends RangeLike> = (source: T, offset: number, length: number) => any;

export function clampIndex<T extends RangeLike>(source: T, index: number): number;
export function clampIndex<T extends RangeLike, N extends ClampThen<T> = ClampThen<T>>(source: T, index: number, then: N): ReturnType<N>;
/** @ignore */
export function clampIndex<T extends RangeLike, N extends ClampThen<T> = ClampThen<T>>(source: T, index: number, then?: N) {
const length = source.length;
const adjust = index > -1 ? index : (length + (index % length));
return then ? then(source, adjust) : adjust;
}

/** @ignore */
let tmp: number;
export function clampRange<T extends RangeLike>(source: T, begin: number | undefined, end: number | undefined): [number, number];
Expand All @@ -60,6 +49,9 @@ export function clampRange<T extends RangeLike, N extends ClampRangeThen<T> = Cl
return then ? then(source, lhs, rhs) : [lhs, rhs];
}

/** @ignore */
export const wrapIndex = (index: number, len: number) => index < 0 ? (len + index) : index;

const isNaNFast = (value: any) => value !== value;

/** @ignore */
Expand Down
10 changes: 9 additions & 1 deletion js/src/vector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
// under the License.

import { Type } from './enum.js';
import { clampRange } from './util/vector.js';
import { clampRange, wrapIndex } from './util/vector.js';
import { DataType, strideForType } from './type.js';
import { Data, makeData, DataProps } from './data.js';
import { BigIntArray, TypedArray, TypedArrayDataType } from './interfaces.js';
Expand Down Expand Up @@ -177,6 +177,14 @@ export class Vector<T extends DataType = any> {
// @ts-ignore
public get(index: number): T['TValue'] | null { return null; }

/**
* Get an element value by position.
* @param index The index of the element to read. A negative index will count back from the last element.
*/
public at(index: number): T['TValue'] | null {
return this.get(wrapIndex(index, this.length));
}

/**
* Set an element value by position.
* @param index The index of the element to write.
Expand Down
7 changes: 4 additions & 3 deletions js/test/unit/vector/bool-vector-tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,10 @@ describe(`BoolVector`, () => {
const n = values.length;
const vector = newBoolVector(n, new Uint8Array([27, 0, 0, 0, 0, 0, 0, 0]));
test(`gets expected values`, () => {
let i = -1;
while (++i < n) {
for (let i = 0; i < values.length; i++) {
expect(vector.get(i)).toEqual(values[i]);
expect(vector.at(i)).toEqual(values.at(i));
expect(vector.at(-i)).toEqual(values.at(-i));
}
});
test(`iterates expected values`, () => {
Expand All @@ -53,7 +54,7 @@ describe(`BoolVector`, () => {
const expected2 = [true, true, true, true, true, false, false, false];
const expected3 = [true, true, false, false, false, false, true, true];
function validate(expected: boolean[]) {
for (let i = -1; ++i < n;) {
for (let i = 0; i < n; i++) {
expect(v.get(i)).toEqual(expected[i]);
}
}
Expand Down
6 changes: 4 additions & 2 deletions js/test/unit/vector/numeric-vector-tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -337,10 +337,12 @@ function testAndValidateVector<T extends Int | Float>(vector: Vector<T>, typed:
function gets_expected_values<T extends Int | Float>(vector: Vector<T>, typed: T['TArray'], values: any[] = [...typed]) {
test(`gets expected values`, () => {
expect.hasAssertions();
let i = -1, n = vector.length;
let i = -1;
try {
while (++i < n) {
while (++i < vector.length) {
expect(vector.get(i)).toEqual(values[i]);
expect(vector.at(i)).toEqual(values.at(i));
expect(vector.at(-i)).toEqual(values.at(-i));
}
} catch (e) { throw new Error(`${i}: ${e}`); }
});
Expand Down
11 changes: 7 additions & 4 deletions js/test/unit/vector/vector-tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -265,8 +265,10 @@ describe(`ListVector`, () => {
});

test(`get value`, () => {
for (const [i, value] of values.entries()) {
expect(vector.get(i)!.toJSON()).toEqual(value);
for (let i = 0; i < values.length; i++) {
expect(vector.get(i)!.toJSON()).toEqual(values[i]);
expect(vector.at(i)!.toJSON()).toEqual(values.at(i));
expect(vector.at(-i)!.toJSON()).toEqual(values.at(-i));
}
});
});
Expand Down Expand Up @@ -308,9 +310,10 @@ function basicVectorTests(vector: Vector, values: any[], extras: any[]) {
const n = values.length;

test(`gets expected values`, () => {
let i = -1;
while (++i < n) {
for (let i = 0; i < values.length; i++) {
expect(vector.get(i)).toEqual(values[i]);
expect(vector.at(i)).toEqual(values.at(i));
expect(vector.at(-i)).toEqual(values.at(-i));
}
});
test(`iterates expected values`, () => {
Expand Down

0 comments on commit 3d690b7

Please sign in to comment.