Copilot commented on code in PR #385:
URL: https://github.com/apache/arrow-js/pull/385#discussion_r2887753873
##########
src/factories.ts:
##########
@@ -80,9 +80,35 @@ export function vectorFromArray<T extends
dtypes.DataType>(data: DataProps<T>):
export function vectorFromArray<T extends TypedArray | BigIntArray | readonly
unknown[]>(data: T): Vector<ArrayDataType<T>>;
export function vectorFromArray(init: any, type?: dtypes.DataType) {
- if (init instanceof Data || init instanceof Vector || init.type instanceof
dtypes.DataType || ArrayBuffer.isView(init)) {
+ if (init instanceof Data || init instanceof Vector || init.type instanceof
dtypes.DataType) {
return makeVector(init as any);
}
+ if (ArrayBuffer.isView(init) && !type) {
+ return makeVector(init as any);
+ }
+ if (ArrayBuffer.isView(init) && type) {
+ // Validate BigInt/number boundary
+ const isBigIntInput = init instanceof BigInt64Array || init instanceof
BigUint64Array;
+ const isBigIntTarget = type.ArrayType === BigInt64Array ||
type.ArrayType === BigUint64Array;
+ if (isBigIntInput && !isBigIntTarget) {
+ throw new TypeError(
+ `Cannot convert BigInt input to ${type}. BigInt arrays can
only target BigInt-based types (e.g. Int64, Uint64).`
+ );
+ }
+ if (!isBigIntInput && isBigIntTarget) {
+ throw new TypeError(
+ `Cannot convert non-BigInt input to ${type}. ${type} requires
BigInt values.`
+ );
+ }
+
+ // Fast path: direct TypedArray conversion for Int and Float types
+ if (dtypes.DataType.isInt(type) || dtypes.DataType.isFloat(type)) {
+ const data = init.constructor === type.ArrayType
+ ? init // zero-copy, same
TypedArray type
+ : new (type.ArrayType as any)(init); // standard JS
TypedArray conversion
+ return makeVector({ type, data, offset: 0, length: data.length,
nullCount: 0 } as any);
+ }
Review Comment:
The fast path for `Float16` type is incorrect when the input TypedArray is
not already a `Uint16Array`. `Float16` uses `Uint16Array` as its `ArrayType`,
but the values stored are IEEE 754 half-precision encoded, not plain integers.
Doing `new Uint16Array(new Float32Array([1.5]))` yields `Uint16Array([1])`, not
the correct half-precision encoding (`0x3E00`).
Consider excluding `Float16` (i.e., `Precision.HALF`) from this fast path so
it falls through to the builder, which correctly handles the float16 encoding.
For example, the condition could additionally check
`!(dtypes.DataType.isFloat(type) && (type as dtypes.Float).precision ===
Precision.HALF)`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
