From 7d166409f4fafe4408776dd318517cedad0c7cce Mon Sep 17 00:00:00 2001 From: Ben Bernays Date: Tue, 8 Aug 2023 15:32:49 +0000 Subject: [PATCH 1/9] add String type --- src/scalar/string.test.ts | 36 +++++++++++++++++++++++++ src/scalar/string.ts | 55 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+) create mode 100644 src/scalar/string.test.ts create mode 100644 src/scalar/string.ts diff --git a/src/scalar/string.test.ts b/src/scalar/string.test.ts new file mode 100644 index 0000000..4723500 --- /dev/null +++ b/src/scalar/string.test.ts @@ -0,0 +1,36 @@ +import { DataType } from '@apache-arrow/esnext-esm'; +import test from 'ava'; + +import { String } from './string.js'; + +// eslint-disable-next-line unicorn/no-null +[null, undefined].forEach((v) => { + test(`should set values to false when ${v} is passed`, (t) => { + const b = new String(v); + t.is(b.Valid, false); + t.true(DataType.isBool(b.DataType)); + }); +}); + +[1, true, 'true', 'Y', 'y', 'TRUE', 'on', new String(true)].forEach((v, index) => { + test(`should support truthy value '${v}' (${index})`, (t) => { + const s = new String(v); + t.is(s.Valid, true); + t.is(s.Value, v.toString()); + t.true(DataType.isUtf8(s.DataType)); + }); +}); + +// [0, false, 'false', 'N', 'n', 'FALSE', 'off'].forEach((v, index) => { +// test(`should support falsy value '${v}' (${index})`, (t) => { +// const b = new String(v); +// t.is(b.Valid, true); +// t.is(b.Value, false); +// t.true(DataType.isBool(b.DataType)); +// t.is(b.toString(), 'false'); +// }); +// }); + +test('should throw when unable to set value', (t) => { + t.throws(() => new String({ value: {} }), { message: "Unable to set '[object Object]' as Bool" }); +}); diff --git a/src/scalar/string.ts b/src/scalar/string.ts new file mode 100644 index 0000000..0a5686b --- /dev/null +++ b/src/scalar/string.ts @@ -0,0 +1,55 @@ +import { Utf8 as ArrowString } from '@apache-arrow/esnext-esm'; +import { boolean, isBooleanable } from 'boolean'; + +import { isInvalid, NULL_VALUE } from './util.js'; + +export class String { + private _valid = false; + private _value = ""; + + public constructor(v: unknown) { + this.Valid = v; + return this; + } + + public get DataType() { + return new ArrowString(); + } + + public get Valid(): boolean { + return this._valid; + } + + public get Value(): string { + return this._value; + } + + public set Valid(value: unknown) { + if (isInvalid(value)) { + this._valid = false; + return; + } + + if (value instanceof String) { + this._valid = value.Valid; + this._value = value.Value; + return; + } + + if (typeof value === 'string' || value instanceof String) { + this._value = this.toString(); + this._valid = true; + return; + } + + throw new Error(`Unable to set '${value}' as String`); + } + + public toString() { + if (this._valid) { + return this._value.toString(); + } + + return NULL_VALUE; + } +} From fb2cac219e70fd0a88cfc3f1cd2ede1dad9b818a Mon Sep 17 00:00:00 2001 From: Ben Bernays Date: Tue, 8 Aug 2023 19:38:18 +0000 Subject: [PATCH 2/9] Add tests for string/text --- .gitignore | 1 + src/scalar/string.test.ts | 36 ------------------------------- src/scalar/text.test.ts | 27 +++++++++++++++++++++++ src/scalar/{string.ts => text.ts} | 20 ++++++----------- 4 files changed, 34 insertions(+), 50 deletions(-) delete mode 100644 src/scalar/string.test.ts create mode 100644 src/scalar/text.test.ts rename src/scalar/{string.ts => text.ts} (67%) diff --git a/.gitignore b/.gitignore index 25eac54..1288698 100644 --- a/.gitignore +++ b/.gitignore @@ -131,3 +131,4 @@ dist # Editor settings .idea +.vscode/launch.json diff --git a/src/scalar/string.test.ts b/src/scalar/string.test.ts deleted file mode 100644 index 4723500..0000000 --- a/src/scalar/string.test.ts +++ /dev/null @@ -1,36 +0,0 @@ -import { DataType } from '@apache-arrow/esnext-esm'; -import test from 'ava'; - -import { String } from './string.js'; - -// eslint-disable-next-line unicorn/no-null -[null, undefined].forEach((v) => { - test(`should set values to false when ${v} is passed`, (t) => { - const b = new String(v); - t.is(b.Valid, false); - t.true(DataType.isBool(b.DataType)); - }); -}); - -[1, true, 'true', 'Y', 'y', 'TRUE', 'on', new String(true)].forEach((v, index) => { - test(`should support truthy value '${v}' (${index})`, (t) => { - const s = new String(v); - t.is(s.Valid, true); - t.is(s.Value, v.toString()); - t.true(DataType.isUtf8(s.DataType)); - }); -}); - -// [0, false, 'false', 'N', 'n', 'FALSE', 'off'].forEach((v, index) => { -// test(`should support falsy value '${v}' (${index})`, (t) => { -// const b = new String(v); -// t.is(b.Valid, true); -// t.is(b.Value, false); -// t.true(DataType.isBool(b.DataType)); -// t.is(b.toString(), 'false'); -// }); -// }); - -test('should throw when unable to set value', (t) => { - t.throws(() => new String({ value: {} }), { message: "Unable to set '[object Object]' as Bool" }); -}); diff --git a/src/scalar/text.test.ts b/src/scalar/text.test.ts new file mode 100644 index 0000000..d8fcf2d --- /dev/null +++ b/src/scalar/text.test.ts @@ -0,0 +1,27 @@ +import { DataType } from '@apache-arrow/esnext-esm'; +import test from 'ava'; + +import { Text } from './text.js'; + +// eslint-disable-next-line unicorn/no-null +[null, undefined].forEach((v) => { + test(`should set values to empty string when ${v} is passed`, (t) => { + const s = new Text(v); + t.is(s.Value, ""); + t.true(DataType.isUtf8(s.DataType)); + }); +}); + +["","test string", new String("asdf")].forEach((v, index) => { + test(`valid strings: '${v}' (${index})`, (t) => { + const s = new Text(v); + t.is(s.Valid, true); + t.is(s.Value, v.toString()); + t.true(DataType.isUtf8(s.DataType)); + }); +}); + + +test('should throw when unable to set value', (t) => { + t.throws(() => new Text({ value: {} }), { message: "Unable to set '[object Object]' as Text" }); +}); diff --git a/src/scalar/string.ts b/src/scalar/text.ts similarity index 67% rename from src/scalar/string.ts rename to src/scalar/text.ts index 0a5686b..6d942b1 100644 --- a/src/scalar/string.ts +++ b/src/scalar/text.ts @@ -1,14 +1,13 @@ import { Utf8 as ArrowString } from '@apache-arrow/esnext-esm'; -import { boolean, isBooleanable } from 'boolean'; import { isInvalid, NULL_VALUE } from './util.js'; -export class String { +export class Text { private _valid = false; private _value = ""; public constructor(v: unknown) { - this.Valid = v; + this.Value = v return this; } @@ -24,25 +23,18 @@ export class String { return this._value; } - public set Valid(value: unknown) { + public set Value(value: unknown) { if (isInvalid(value)) { this._valid = false; return; } - - if (value instanceof String) { - this._valid = value.Valid; - this._value = value.Value; - return; - } - if (typeof value === 'string' || value instanceof String) { - this._value = this.toString(); + this._value = value.toString(); this._valid = true; return; } - - throw new Error(`Unable to set '${value}' as String`); + + throw new Error(`Unable to set '${value}' as Text`); } public toString() { From 205dff601b7c288ac36b31877646eabb69da024e Mon Sep 17 00:00:00 2001 From: Ben Bernays Date: Tue, 8 Aug 2023 19:41:42 +0000 Subject: [PATCH 3/9] linting and formatting --- src/scalar/text.test.ts | 5 ++--- src/scalar/text.ts | 6 +++--- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/scalar/text.test.ts b/src/scalar/text.test.ts index d8fcf2d..7647150 100644 --- a/src/scalar/text.test.ts +++ b/src/scalar/text.test.ts @@ -7,12 +7,12 @@ import { Text } from './text.js'; [null, undefined].forEach((v) => { test(`should set values to empty string when ${v} is passed`, (t) => { const s = new Text(v); - t.is(s.Value, ""); + t.is(s.Value, ''); t.true(DataType.isUtf8(s.DataType)); }); }); -["","test string", new String("asdf")].forEach((v, index) => { +['', 'test string', String('asdf')].forEach((v, index) => { test(`valid strings: '${v}' (${index})`, (t) => { const s = new Text(v); t.is(s.Valid, true); @@ -21,7 +21,6 @@ import { Text } from './text.js'; }); }); - test('should throw when unable to set value', (t) => { t.throws(() => new Text({ value: {} }), { message: "Unable to set '[object Object]' as Text" }); }); diff --git a/src/scalar/text.ts b/src/scalar/text.ts index 6d942b1..605e187 100644 --- a/src/scalar/text.ts +++ b/src/scalar/text.ts @@ -4,10 +4,10 @@ import { isInvalid, NULL_VALUE } from './util.js'; export class Text { private _valid = false; - private _value = ""; + private _value = ''; public constructor(v: unknown) { - this.Value = v + this.Value = v; return this; } @@ -33,7 +33,7 @@ export class Text { this._valid = true; return; } - + throw new Error(`Unable to set '${value}' as Text`); } From c46377b9b086292db120605d5f4ca8568d470430 Mon Sep 17 00:00:00 2001 From: Ben Bernays Date: Tue, 8 Aug 2023 19:50:48 +0000 Subject: [PATCH 4/9] linting --- src/scalar/text.test.ts | 10 +++++----- src/scalar/text.ts | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/scalar/text.test.ts b/src/scalar/text.test.ts index 7647150..dd7a17a 100644 --- a/src/scalar/text.test.ts +++ b/src/scalar/text.test.ts @@ -7,17 +7,17 @@ import { Text } from './text.js'; [null, undefined].forEach((v) => { test(`should set values to empty string when ${v} is passed`, (t) => { const s = new Text(v); - t.is(s.Value, ''); - t.true(DataType.isUtf8(s.DataType)); + t.is(s.value, ''); + t.true(DataType.isUtf8(s.dataType)); }); }); ['', 'test string', String('asdf')].forEach((v, index) => { test(`valid strings: '${v}' (${index})`, (t) => { const s = new Text(v); - t.is(s.Valid, true); - t.is(s.Value, v.toString()); - t.true(DataType.isUtf8(s.DataType)); + t.is(s.valid, true); + t.is(s.value, v.toString()); + t.true(DataType.isUtf8(s.dataType)); }); }); diff --git a/src/scalar/text.ts b/src/scalar/text.ts index 605e187..415283b 100644 --- a/src/scalar/text.ts +++ b/src/scalar/text.ts @@ -7,23 +7,23 @@ export class Text { private _value = ''; public constructor(v: unknown) { - this.Value = v; + this.value = v; return this; } - public get DataType() { + public get dataType() { return new ArrowString(); } - public get Valid(): boolean { + public get valid(): boolean { return this._valid; } - public get Value(): string { + public get value(): string { return this._value; } - public set Value(value: unknown) { + public set value(value: unknown) { if (isInvalid(value)) { this._valid = false; return; From 9291ac975a9b8e486631eacde9dc9e5c2d84c2f3 Mon Sep 17 00:00:00 2001 From: Ben Bernays Date: Wed, 9 Aug 2023 12:23:58 +0000 Subject: [PATCH 5/9] Updated tsconfig to access Object`.hasOwn` --- tsconfig.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tsconfig.json b/tsconfig.json index 995e24e..9444c14 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -4,6 +4,7 @@ "allowJs": true, "declaration": true, "outDir": "dist", - "sourceMap": true + "sourceMap": true, + "lib": ["esnext", "dom"] } } From 343c39ffb52275742655badb3364943288e3f263 Mon Sep 17 00:00:00 2001 From: Ben Bernays Date: Wed, 9 Aug 2023 12:24:18 +0000 Subject: [PATCH 6/9] tests passing and pr review comments --- src/scalar/text.test.ts | 3 ++- src/scalar/text.ts | 31 ++++++++++++++++++++++--------- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/src/scalar/text.test.ts b/src/scalar/text.test.ts index dd7a17a..5638530 100644 --- a/src/scalar/text.test.ts +++ b/src/scalar/text.test.ts @@ -8,11 +8,12 @@ import { Text } from './text.js'; test(`should set values to empty string when ${v} is passed`, (t) => { const s = new Text(v); t.is(s.value, ''); + t.is(s.valid, false); t.true(DataType.isUtf8(s.dataType)); }); }); -['', 'test string', String('asdf')].forEach((v, index) => { +['', 'test string', String('new string object'), new Text('new text object')].forEach((v, index) => { test(`valid strings: '${v}' (${index})`, (t) => { const s = new Text(v); t.is(s.valid, true); diff --git a/src/scalar/text.ts b/src/scalar/text.ts index 415283b..5733000 100644 --- a/src/scalar/text.ts +++ b/src/scalar/text.ts @@ -1,8 +1,9 @@ import { Utf8 as ArrowString } from '@apache-arrow/esnext-esm'; +import { Scalar } from './scalar.js'; import { isInvalid, NULL_VALUE } from './util.js'; -export class Text { +export class Text implements Scalar { private _valid = false; private _value = ''; @@ -28,16 +29,28 @@ export class Text { this._valid = false; return; } - if (typeof value === 'string' || value instanceof String) { - this._value = value.toString(); - this._valid = true; - return; - } - throw new Error(`Unable to set '${value}' as Text`); + switch (typeof value) { + case 'object': + if (value !== undefined && + value !== null && + Object.hasOwn(value,'toString')) { + this._value = value.toString(); + this._valid = true; + return; + } + case 'string': + if (typeof value === 'string' || value instanceof String) { + this._value = value.toString(); + this._valid = true; + return; + } + default: + throw new Error(`Unable to set '${value}' as Text`); + } } - - public toString() { + + public toString = () : string => { if (this._valid) { return this._value.toString(); } From b9680742c3f7e37a6db12fac0b1df0790c2fd235 Mon Sep 17 00:00:00 2001 From: Ben Bernays Date: Wed, 9 Aug 2023 12:25:37 +0000 Subject: [PATCH 7/9] linting --- src/scalar/text.ts | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/scalar/text.ts b/src/scalar/text.ts index 5733000..acd48d5 100644 --- a/src/scalar/text.ts +++ b/src/scalar/text.ts @@ -32,13 +32,11 @@ export class Text implements Scalar { switch (typeof value) { case 'object': - if (value !== undefined && - value !== null && - Object.hasOwn(value,'toString')) { + if (value !== undefined && value !== null && Object.hasOwn(value, 'toString')) { this._value = value.toString(); this._valid = true; return; - } + } case 'string': if (typeof value === 'string' || value instanceof String) { this._value = value.toString(); @@ -47,14 +45,14 @@ export class Text implements Scalar { } default: throw new Error(`Unable to set '${value}' as Text`); - } + } } - - public toString = () : string => { + + public toString = (): string => { if (this._valid) { return this._value.toString(); } return NULL_VALUE; - } + }; } From 8708186c4b42d92a399ac18081a9d2c0a71e66e7 Mon Sep 17 00:00:00 2001 From: Ben Bernays Date: Wed, 9 Aug 2023 12:30:43 +0000 Subject: [PATCH 8/9] linting/formatting --- src/scalar/text.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/scalar/text.ts b/src/scalar/text.ts index acd48d5..235dd6f 100644 --- a/src/scalar/text.ts +++ b/src/scalar/text.ts @@ -31,21 +31,21 @@ export class Text implements Scalar { } switch (typeof value) { - case 'object': + case 'object': { if (value !== undefined && value !== null && Object.hasOwn(value, 'toString')) { this._value = value.toString(); this._valid = true; return; } - case 'string': - if (typeof value === 'string' || value instanceof String) { - this._value = value.toString(); - this._valid = true; - return; - } - default: - throw new Error(`Unable to set '${value}' as Text`); + break; + } + case 'string': { + this._value = value.toString(); + this._valid = true; + return; + } } + throw new Error(`Unable to set '${value}' as Text`); } public toString = (): string => { From d48bf8a74f37939c178eefae0288512096578323 Mon Sep 17 00:00:00 2001 From: erezrokah Date: Thu, 10 Aug 2023 16:14:48 +0200 Subject: [PATCH 9/9] fix: Review comments --- .gitignore | 2 +- src/scalar/text.test.ts | 14 ++++++++++---- src/scalar/text.ts | 35 +++++++++++++++++++---------------- tsconfig.json | 3 +-- 4 files changed, 31 insertions(+), 23 deletions(-) diff --git a/.gitignore b/.gitignore index 1288698..4e00445 100644 --- a/.gitignore +++ b/.gitignore @@ -131,4 +131,4 @@ dist # Editor settings .idea -.vscode/launch.json +.vscode diff --git a/src/scalar/text.test.ts b/src/scalar/text.test.ts index 5638530..dee83d9 100644 --- a/src/scalar/text.test.ts +++ b/src/scalar/text.test.ts @@ -13,11 +13,17 @@ import { Text } from './text.js'; }); }); -['', 'test string', String('new string object'), new Text('new text object')].forEach((v, index) => { - test(`valid strings: '${v}' (${index})`, (t) => { - const s = new Text(v); +[ + { value: '', expected: '' }, + { value: 'test string', expected: 'test string' }, + { value: String('new string object'), expected: 'new string object' }, + { value: new Text('new text object'), expected: 'new text object' }, + { value: new TextEncoder().encode('test'), expected: 'test' }, +].forEach(({ value, expected }, index) => { + test(`valid strings: '${value}' (${index})`, (t) => { + const s = new Text(value); t.is(s.valid, true); - t.is(s.value, v.toString()); + t.is(s.value, expected); t.true(DataType.isUtf8(s.dataType)); }); }); diff --git a/src/scalar/text.ts b/src/scalar/text.ts index 235dd6f..deb4e41 100644 --- a/src/scalar/text.ts +++ b/src/scalar/text.ts @@ -30,29 +30,32 @@ export class Text implements Scalar { return; } - switch (typeof value) { - case 'object': { - if (value !== undefined && value !== null && Object.hasOwn(value, 'toString')) { - this._value = value.toString(); - this._valid = true; - return; - } - break; - } - case 'string': { - this._value = value.toString(); - this._valid = true; - return; - } + if (typeof value === 'string') { + this._value = value; + this._valid = true; + return; } + + if (value instanceof Uint8Array) { + this._value = new TextDecoder().decode(value); + this._valid = true; + return; + } + + if (typeof value!.toString === 'function' && value!.toString !== Object.prototype.toString) { + this._value = value!.toString(); + this._valid = true; + return; + } + throw new Error(`Unable to set '${value}' as Text`); } - public toString = (): string => { + public toString() { if (this._valid) { return this._value.toString(); } return NULL_VALUE; - }; + } } diff --git a/tsconfig.json b/tsconfig.json index 9444c14..995e24e 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -4,7 +4,6 @@ "allowJs": true, "declaration": true, "outDir": "dist", - "sourceMap": true, - "lib": ["esnext", "dom"] + "sourceMap": true } }