TheNeuralBit commented on a change in pull request #8216:
URL: https://github.com/apache/arrow/pull/8216#discussion_r492096757
##########
File path: js/src/interfaces.ts
##########
@@ -356,48 +356,48 @@ type TypeToBuilder<T extends Type = any, TNull = any> = {
/** @ignore */
type DataTypeToBuilder<T extends DataType = any, TNull = any> = {
- [key: number ]:
builders.Builder<any,
TNull> ;
- [Type.Null ]: T extends type.Null ?
builders.NullBuilder<TNull> :
builders.Builder<any, TNull> ;
- [Type.Bool ]: T extends type.Bool ?
builders.BoolBuilder<TNull> :
builders.Builder<any, TNull> ;
- [Type.Int8 ]: T extends type.Int8 ?
builders.Int8Builder<TNull> :
builders.Builder<any, TNull> ;
- [Type.Int16 ]: T extends type.Int16 ?
builders.Int16Builder<TNull> :
builders.Builder<any, TNull> ;
- [Type.Int32 ]: T extends type.Int32 ?
builders.Int32Builder<TNull> :
builders.Builder<any, TNull> ;
- [Type.Int64 ]: T extends type.Int64 ?
builders.Int64Builder<TNull> :
builders.Builder<any, TNull> ;
- [Type.Uint8 ]: T extends type.Uint8 ?
builders.Uint8Builder<TNull> :
builders.Builder<any, TNull> ;
- [Type.Uint16 ]: T extends type.Uint16 ?
builders.Uint16Builder<TNull> :
builders.Builder<any, TNull> ;
- [Type.Uint32 ]: T extends type.Uint32 ?
builders.Uint32Builder<TNull> :
builders.Builder<any, TNull> ;
- [Type.Uint64 ]: T extends type.Uint64 ?
builders.Uint64Builder<TNull> :
builders.Builder<any, TNull> ;
- [Type.Int ]: T extends type.Int ?
builders.IntBuilder<T, TNull> :
builders.Builder<any, TNull> ;
- [Type.Float16 ]: T extends type.Float16 ?
builders.Float16Builder<TNull> :
builders.Builder<any, TNull> ;
- [Type.Float32 ]: T extends type.Float32 ?
builders.Float32Builder<TNull> :
builders.Builder<any, TNull> ;
- [Type.Float64 ]: T extends type.Float64 ?
builders.Float64Builder<TNull> :
builders.Builder<any, TNull> ;
- [Type.Float ]: T extends type.Float ?
builders.FloatBuilder<T, TNull> :
builders.Builder<any, TNull> ;
- [Type.Utf8 ]: T extends type.Utf8 ?
builders.Utf8Builder<TNull> :
builders.Builder<any, TNull> ;
- [Type.Binary ]: T extends type.Binary ?
builders.BinaryBuilder<TNull> :
builders.Builder<any, TNull> ;
- [Type.FixedSizeBinary ]: T extends type.FixedSizeBinary ?
builders.FixedSizeBinaryBuilder<TNull> :
builders.Builder<any, TNull> ;
- [Type.Date ]: T extends type.Date_ ?
builders.DateBuilder<T, TNull> :
builders.Builder<any, TNull> ;
- [Type.DateDay ]: T extends type.DateDay ?
builders.DateDayBuilder<TNull> :
builders.Builder<any, TNull> ;
- [Type.DateMillisecond ]: T extends type.DateMillisecond ?
builders.DateMillisecondBuilder<TNull> :
builders.Builder<any, TNull> ;
- [Type.Timestamp ]: T extends type.Timestamp ?
builders.TimestampBuilder<T, TNull> :
builders.Builder<any, TNull> ;
- [Type.TimestampSecond ]: T extends type.TimestampSecond ?
builders.TimestampSecondBuilder<TNull> :
builders.Builder<any, TNull> ;
- [Type.TimestampMillisecond ]: T extends type.TimestampMillisecond ?
builders.TimestampMillisecondBuilder<TNull> :
builders.Builder<any, TNull> ;
- [Type.TimestampMicrosecond ]: T extends type.TimestampMicrosecond ?
builders.TimestampMicrosecondBuilder<TNull> :
builders.Builder<any, TNull> ;
- [Type.TimestampNanosecond ]: T extends type.TimestampNanosecond ?
builders.TimestampNanosecondBuilder<TNull> :
builders.Builder<any, TNull> ;
- [Type.Time ]: T extends type.Time ?
builders.TimeBuilder<T, TNull> :
builders.Builder<any, TNull> ;
- [Type.TimeSecond ]: T extends type.TimeSecond ?
builders.TimeSecondBuilder<TNull> :
builders.Builder<any, TNull> ;
- [Type.TimeMillisecond ]: T extends type.TimeMillisecond ?
builders.TimeMillisecondBuilder<TNull> :
builders.Builder<any, TNull> ;
- [Type.TimeMicrosecond ]: T extends type.TimeMicrosecond ?
builders.TimeMicrosecondBuilder<TNull> :
builders.Builder<any, TNull> ;
- [Type.TimeNanosecond ]: T extends type.TimeNanosecond ?
builders.TimeNanosecondBuilder<TNull> :
builders.Builder<any, TNull> ;
- [Type.Decimal ]: T extends type.Decimal ?
builders.DecimalBuilder<TNull> :
builders.Builder<any, TNull> ;
- [Type.Union ]: T extends type.Union ?
builders.UnionBuilder<T, TNull> :
builders.Builder<any, TNull> ;
- [Type.DenseUnion ]: T extends type.DenseUnion ?
builders.DenseUnionBuilder<T, TNull> :
builders.Builder<any, TNull> ;
- [Type.SparseUnion ]: T extends type.SparseUnion ?
builders.SparseUnionBuilder<T, TNull> :
builders.Builder<any, TNull> ;
- [Type.Interval ]: T extends type.Interval ?
builders.IntervalBuilder<T, TNull> :
builders.Builder<any, TNull> ;
- [Type.IntervalDayTime ]: T extends type.IntervalDayTime ?
builders.IntervalDayTimeBuilder<TNull> :
builders.Builder<any, TNull> ;
- [Type.IntervalYearMonth ]: T extends type.IntervalYearMonth ?
builders.IntervalYearMonthBuilder<TNull> :
builders.Builder<any, TNull> ;
- [Type.Map ]: T extends type.Map_ ?
builders.MapBuilder<T['keyType'], T['valueType'], TNull> :
builders.Builder<any, TNull> ;
- [Type.List ]: T extends type.List ?
builders.ListBuilder<T['valueType'], TNull> :
builders.Builder<any, TNull> ;
- [Type.Struct ]: T extends type.Struct ?
builders.StructBuilder<T['dataTypes'], TNull> :
builders.Builder<any, TNull> ;
- [Type.Dictionary ]: T extends type.Dictionary ?
builders.DictionaryBuilder<T, TNull> :
builders.Builder<any, TNull> ;
- [Type.FixedSizeList ]: T extends type.FixedSizeList ?
builders.FixedSizeListBuilder<T['valueType'], TNull> :
builders.Builder<any, TNull> ;
+ [key: number ]:
builders.Builder<any, TNull> ;
+ [Type.Null ]: T extends type.Null ?
builders.NullBuilder<TNull> : never ;
+ [Type.Bool ]: T extends type.Bool ?
builders.BoolBuilder<TNull> : never ;
+ [Type.Int8 ]: T extends type.Int8 ?
builders.Int8Builder<TNull> : never ;
+ [Type.Int16 ]: T extends type.Int16 ?
builders.Int16Builder<TNull> : never ;
+ [Type.Int32 ]: T extends type.Int32 ?
builders.Int32Builder<TNull> : never ;
+ [Type.Int64 ]: T extends type.Int64 ?
builders.Int64Builder<TNull> : never ;
+ [Type.Uint8 ]: T extends type.Uint8 ?
builders.Uint8Builder<TNull> : never ;
+ [Type.Uint16 ]: T extends type.Uint16 ?
builders.Uint16Builder<TNull> : never ;
+ [Type.Uint32 ]: T extends type.Uint32 ?
builders.Uint32Builder<TNull> : never ;
+ [Type.Uint64 ]: T extends type.Uint64 ?
builders.Uint64Builder<TNull> : never ;
+ [Type.Int ]: T extends type.Int ?
builders.IntBuilder<T, TNull> : never ;
+ [Type.Float16 ]: T extends type.Float16 ?
builders.Float16Builder<TNull> : never ;
+ [Type.Float32 ]: T extends type.Float32 ?
builders.Float32Builder<TNull> : never ;
+ [Type.Float64 ]: T extends type.Float64 ?
builders.Float64Builder<TNull> : never ;
+ [Type.Float ]: T extends type.Float ?
builders.FloatBuilder<T, TNull> : never ;
+ [Type.Utf8 ]: T extends type.Utf8 ?
builders.Utf8Builder<TNull> : never ;
+ [Type.Binary ]: T extends type.Binary ?
builders.BinaryBuilder<TNull> : never ;
+ [Type.FixedSizeBinary ]: T extends type.FixedSizeBinary ?
builders.FixedSizeBinaryBuilder<TNull> : never ;
+ [Type.Date ]: T extends type.Date_ ?
builders.DateBuilder<T, TNull> : never ;
+ [Type.DateDay ]: T extends type.DateDay ?
builders.DateDayBuilder<TNull> : never ;
+ [Type.DateMillisecond ]: T extends type.DateMillisecond ?
builders.DateMillisecondBuilder<TNull> : never ;
+ [Type.Timestamp ]: T extends type.Timestamp ?
builders.TimestampBuilder<T, TNull> : never ;
+ [Type.TimestampSecond ]: T extends type.TimestampSecond ?
builders.TimestampSecondBuilder<TNull> : never ;
+ [Type.TimestampMillisecond ]: T extends type.TimestampMillisecond ?
builders.TimestampMillisecondBuilder<TNull> : never ;
+ [Type.TimestampMicrosecond ]: T extends type.TimestampMicrosecond ?
builders.TimestampMicrosecondBuilder<TNull> : never ;
+ [Type.TimestampNanosecond ]: T extends type.TimestampNanosecond ?
builders.TimestampNanosecondBuilder<TNull> : never ;
+ [Type.Time ]: T extends type.Time ?
builders.TimeBuilder<T, TNull> : never ;
+ [Type.TimeSecond ]: T extends type.TimeSecond ?
builders.TimeSecondBuilder<TNull> : never ;
+ [Type.TimeMillisecond ]: T extends type.TimeMillisecond ?
builders.TimeMillisecondBuilder<TNull> : never ;
+ [Type.TimeMicrosecond ]: T extends type.TimeMicrosecond ?
builders.TimeMicrosecondBuilder<TNull> : never ;
+ [Type.TimeNanosecond ]: T extends type.TimeNanosecond ?
builders.TimeNanosecondBuilder<TNull> : never ;
+ [Type.Decimal ]: T extends type.Decimal ?
builders.DecimalBuilder<TNull> : never ;
+ [Type.Union ]: T extends type.Union ?
builders.UnionBuilder<T, TNull> : never ;
+ [Type.DenseUnion ]: T extends type.DenseUnion ?
builders.DenseUnionBuilder<T, TNull> : never ;
+ [Type.SparseUnion ]: T extends type.SparseUnion ?
builders.SparseUnionBuilder<T, TNull> : never ;
+ [Type.Interval ]: T extends type.Interval ?
builders.IntervalBuilder<T, TNull> : never ;
+ [Type.IntervalDayTime ]: T extends type.IntervalDayTime ?
builders.IntervalDayTimeBuilder<TNull> : never ;
+ [Type.IntervalYearMonth ]: T extends type.IntervalYearMonth ?
builders.IntervalYearMonthBuilder<TNull> : never ;
+ [Type.Map ]: T extends type.Map_ ?
builders.MapBuilder<T['keyType'], T['valueType'], TNull> : never ;
+ [Type.List ]: T extends type.List ?
builders.ListBuilder<T['valueType'], TNull> : never ;
+ [Type.Struct ]: T extends type.Struct ?
builders.StructBuilder<T['dataTypes'], TNull> : never ;
+ [Type.Dictionary ]: T extends type.Dictionary ?
builders.DictionaryBuilder<T, TNull> : never ;
+ [Type.FixedSizeList ]: T extends type.FixedSizeList ?
builders.FixedSizeListBuilder<T['valueType'], TNull> : never ;
Review comment:
Could you explain what this change is for? It looks like it's removing a
fallback but maybe I'm misremembering how this works.
##########
File path: js/test/Arrow.ts
##########
@@ -27,6 +27,13 @@ require('web-stream-tools');
(<any> global).window = (<any> global).window || global;
// Fix for Jest in node v10.x
+Object.defineProperty(Object, Symbol.hasInstance, {
+ writable: true,
+ configurable: true,
+ value(inst: any) {
+ return inst && inst.constructor && inst.constructor.name === 'Object';
+ }
+});
Review comment:
Is this now "Fixes for Jest in node v14.x"?
##########
File path: js/test/unit/ipc/helpers.ts
##########
@@ -54,13 +54,13 @@ export abstract class ArrowIOTestHelper {
await testFn(await this.writer(this.table).toUint8Array());
};
}
- iterable(testFn: (iterable: Iterable<Uint8Array>) => void | Promise<void>)
{
+ iterable(testFn: (iterable: Generator<Uint8Array>) => void |
Promise<void>) {
return async () => {
expect.hasAssertions();
await testFn(chunkedIterable(await
this.writer(this.table).toUint8Array()));
};
}
- asyncIterable(testFn: (asyncIterable: AsyncIterable<Uint8Array>) => void |
Promise<void>) {
+ asyncIterable(testFn: (asyncIterable: AsyncGenerator<Uint8Array>) => void
| Promise<void>) {
Review comment:
This PR changes lots of Iterable and Iterator types to Generator, but
the corresponding names still reference "iterable" or "iterator" Is that
potentially misleading? I'll admit I'm out of the loop on recent changes in JS,
maybe this is common practice
##########
File path: js/test/inference/column.ts
##########
@@ -33,33 +33,6 @@ const boolColumn = new Column(new Field('bool', boolType), [
expect(typeof boolVector.get(0) === 'boolean').toBe(true);
expect(typeof boolColumn.get(0) === 'boolean').toBe(true);
-type NamedSchema = {
- a: Int8;
- b: Utf8;
- c: Dictionary<List<Bool>>;
-};
-
-const mapChildFields = [
- { name: 'a', type: new Int8() },
- { name: 'b', type: new Utf8() },
- { name: 'c', type: new Dictionary<List<Bool>>(null!, null!) }
-].map(({ name, type }) => new Field(name, type));
-
-const mapType = new Map_<NamedSchema>(mapChildFields);
-
-const mapVector = Vector.new(Data.Map(mapType, 0, 0, 0, null, []));
-const mapColumn = new Column(new Field('map', mapType, false), [
- Vector.new(Data.Map(mapType, 0, 0, 0, null, [])),
- Vector.new(Data.Map(mapType, 0, 0, 0, null, [])),
- Vector.new(Data.Map(mapType, 0, 0, 0, null, [])),
-]);
-
-const { a: a1, b: b1, c: c1 } = mapVector.get(0)!;
-const { a: a2, b: b2, c: c2 } = mapColumn.get(0)!;
-
-console.log(a1, b1, c1);
-console.log(a2, b2, c2);
-
Review comment:
I'm a little perplexed by this. It looks like you're removing test code
that's not actually verifying anything, but the test case below isn't verifying
anything either. Should these tests just have some assertions instead?
##########
File path: js/src/data.ts
##########
@@ -253,11 +254,11 @@ export class Data<T extends DataType = DataType> {
return new Data(type, offset, length, nullCount, [undefined,
toArrayBufferView(type.ArrayType, data), toUint8Array(nullBitmap)]);
}
/** @nocollapse */
- public static Binary<T extends Binary>(type: T, offset: number, length:
number, nullCount: number, nullBitmap: NullBuffer, valueOffsets:
ValueOffsetsBuffer, data: Uint8Array) {
+ public static Binary<T extends Binary>(type: T, offset: number, length:
number, nullCount: number, nullBitmap: NullBuffer, valueOffsets:
ValueOffsetsBuffer, data: DataBuffer<T>) {
return new Data(type, offset, length, nullCount,
[toInt32Array(valueOffsets), toUint8Array(data), toUint8Array(nullBitmap)]);
}
/** @nocollapse */
- public static Utf8<T extends Utf8>(type: T, offset: number, length:
number, nullCount: number, nullBitmap: NullBuffer, valueOffsets:
ValueOffsetsBuffer, data: Uint8Array) {
+ public static Utf8<T extends Utf8>(type: T, offset: number, length:
number, nullCount: number, nullBitmap: NullBuffer, valueOffsets:
ValueOffsetsBuffer, data: DataBuffer<T>) {
Review comment:
It looks like this is just correcting an oversight?
##########
File path: js/package.json
##########
@@ -100,11 +100,11 @@
"source-map-loader": "0.2.4",
"terser-webpack-plugin": "1.2.1",
"trash": "4.3.0",
- "ts-jest": "24.0.2",
- "ts-node": "8.2.0",
- "tslint": "5.12.1",
- "typedoc": "0.15.0-0",
- "typescript": "3.5.1",
+ "ts-jest": "26.3.0",
+ "ts-node": "9.0.0",
+ "tslint": "6.1.3",
+ "typedoc": "0.19.1",
+ "typescript": "4.0.2",
Review comment:
`npm audit` says we have 34k vulnerabilities, would you mind addressing
them while we're changing deps? It looks like `npm audit fix` will correct all
but 10 of them.
##########
File path: .env
##########
@@ -30,7 +30,7 @@ LLVM=10
CLANG_TOOLS=8
RUST=nightly-2020-04-22
GO=1.12
-NODE=11
+NODE=14
Review comment:
Does this PR change the minimum supported node version?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
