Skip to content

Commit 045cdb3

Browse files
authored
Merge pull request #14 from githubnext/copilot/aw-manual-apply
Improve Dtype + Series: widened signatures, lint fixes, and removed unsafe casts
2 parents 4130980 + 9cc05c8 commit 045cdb3

5 files changed

Lines changed: 86 additions & 67 deletions

File tree

src/core/dtype.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,10 @@ export class Dtype {
105105
return new Dtype("timedelta", "timedelta", 8);
106106
case "category":
107107
return new Dtype("category", "category", 0);
108+
default: {
109+
const _exhaustive: never = name;
110+
throw new Error(`Unknown dtype: ${_exhaustive}`);
111+
}
108112
}
109113
}
110114

src/core/series.ts

Lines changed: 58 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,36 @@ function defaultIndex(n: number): Index<Label> {
1919
return new RangeIndex(n) as unknown as Index<Label>;
2020
}
2121

22+
/** True when the value should be treated as missing (null, undefined, or NaN). */
23+
function isMissing(v: Scalar): boolean {
24+
return v === null || v === undefined || (typeof v === "number" && Number.isNaN(v));
25+
}
26+
27+
/** Compare two scalar values with null/NaN handling for sorting. */
28+
function compareScalars(
29+
a: Scalar,
30+
b: Scalar,
31+
ascending: boolean,
32+
naPosition: "first" | "last",
33+
): number {
34+
const aNull = isMissing(a);
35+
const bNull = isMissing(b);
36+
if (aNull && bNull) {
37+
return 0;
38+
}
39+
if (aNull) {
40+
return naPosition === "first" ? -1 : 1;
41+
}
42+
if (bNull) {
43+
return naPosition === "first" ? 1 : -1;
44+
}
45+
if (a === b) {
46+
return 0;
47+
}
48+
const cmp = (a as number | string | boolean) < (b as number | string | boolean) ? -1 : 1;
49+
return ascending ? cmp : -cmp;
50+
}
51+
2252
// ─── SeriesOptions ────────────────────────────────────────────────────────────
2353

2454
/** Constructor options accepted by `Series`. */
@@ -184,7 +214,10 @@ export class Series<T extends Scalar = Scalar> {
184214

185215
// ─── arithmetic ───────────────────────────────────────────────────────────
186216

187-
private _scalarOp(other: T | Series<T>, fn: (a: number, b: number) => number): Series<number> {
217+
private _scalarOp(
218+
other: number | Series<Scalar>,
219+
fn: (a: number, b: number) => number,
220+
): Series<number> {
188221
const isScalar = !(other instanceof Series);
189222
if (isScalar) {
190223
const b = other as number;
@@ -199,7 +232,7 @@ export class Series<T extends Scalar = Scalar> {
199232
name: this.name,
200233
});
201234
}
202-
const o = other as Series<T>;
235+
const o = other as Series<Scalar>;
203236
if (o.size !== this.size) {
204237
throw new RangeError(
205238
`Cannot operate on Series of different sizes: ${this.size} vs ${o.size}`,
@@ -219,46 +252,49 @@ export class Series<T extends Scalar = Scalar> {
219252
}
220253

221254
/** Element-wise addition. */
222-
add(other: T | Series<T>): Series<number> {
255+
add(other: number | Series<Scalar>): Series<number> {
223256
return this._scalarOp(other, (a, b) => a + b);
224257
}
225258

226259
/** Element-wise subtraction. */
227-
sub(other: T | Series<T>): Series<number> {
260+
sub(other: number | Series<Scalar>): Series<number> {
228261
return this._scalarOp(other, (a, b) => a - b);
229262
}
230263

231264
/** Element-wise multiplication. */
232-
mul(other: T | Series<T>): Series<number> {
265+
mul(other: number | Series<Scalar>): Series<number> {
233266
return this._scalarOp(other, (a, b) => a * b);
234267
}
235268

236269
/** Element-wise division (true division, returns float). */
237-
div(other: T | Series<T>): Series<number> {
270+
div(other: number | Series<Scalar>): Series<number> {
238271
return this._scalarOp(other, (a, b) => a / b);
239272
}
240273

241274
/** Element-wise floor division. */
242-
floordiv(other: T | Series<T>): Series<number> {
275+
floordiv(other: number | Series<Scalar>): Series<number> {
243276
return this._scalarOp(other, (a, b) => Math.floor(a / b));
244277
}
245278

246279
/** Element-wise modulo. */
247-
mod(other: T | Series<T>): Series<number> {
280+
mod(other: number | Series<Scalar>): Series<number> {
248281
return this._scalarOp(other, (a, b) => a % b);
249282
}
250283

251284
/** Element-wise exponentiation. */
252-
pow(other: T | Series<T>): Series<number> {
285+
pow(other: number | Series<Scalar>): Series<number> {
253286
return this._scalarOp(other, (a, b) => a ** b);
254287
}
255288

256289
// ─── comparison ───────────────────────────────────────────────────────────
257290

258-
private _cmpOp(other: T | Series<T>, fn: (a: T, b: T) => boolean): Series<boolean> {
291+
private _cmpOp(
292+
other: Scalar | Series<Scalar>,
293+
fn: (a: Scalar, b: Scalar) => boolean,
294+
): Series<boolean> {
259295
const isScalar = !(other instanceof Series);
260296
if (isScalar) {
261-
const b = other as T;
297+
const b = other as Scalar;
262298
const newData = this._values.map((a) => fn(a, b));
263299
return new Series<boolean>({
264300
data: newData,
@@ -267,11 +303,11 @@ export class Series<T extends Scalar = Scalar> {
267303
name: this.name,
268304
});
269305
}
270-
const o = other as Series<T>;
306+
const o = other as Series<Scalar>;
271307
if (o.size !== this.size) {
272308
throw new RangeError(`Cannot compare Series of different sizes: ${this.size} vs ${o.size}`);
273309
}
274-
const newData = this._values.map((a, i) => fn(a, o._values[i] as T));
310+
const newData = this._values.map((a, i) => fn(a, o._values[i] as Scalar));
275311
return new Series<boolean>({
276312
data: newData,
277313
index: this.index,
@@ -280,15 +316,15 @@ export class Series<T extends Scalar = Scalar> {
280316
});
281317
}
282318

283-
eq(other: T | Series<T>): Series<boolean> {
319+
eq(other: Scalar | Series<Scalar>): Series<boolean> {
284320
return this._cmpOp(other, (a, b) => a === b);
285321
}
286322

287-
ne(other: T | Series<T>): Series<boolean> {
323+
ne(other: Scalar | Series<Scalar>): Series<boolean> {
288324
return this._cmpOp(other, (a, b) => a !== b);
289325
}
290326

291-
lt(other: T | Series<T>): Series<boolean> {
327+
lt(other: Scalar | Series<Scalar>): Series<boolean> {
292328
return this._cmpOp(other, (a, b) => {
293329
if (a === null || b === null) {
294330
return false;
@@ -297,7 +333,7 @@ export class Series<T extends Scalar = Scalar> {
297333
});
298334
}
299335

300-
le(other: T | Series<T>): Series<boolean> {
336+
le(other: Scalar | Series<Scalar>): Series<boolean> {
301337
return this._cmpOp(other, (a, b) => {
302338
if (a === null || b === null) {
303339
return false;
@@ -306,7 +342,7 @@ export class Series<T extends Scalar = Scalar> {
306342
});
307343
}
308344

309-
gt(other: T | Series<T>): Series<boolean> {
345+
gt(other: Scalar | Series<Scalar>): Series<boolean> {
310346
return this._cmpOp(other, (a, b) => {
311347
if (a === null || b === null) {
312348
return false;
@@ -315,7 +351,7 @@ export class Series<T extends Scalar = Scalar> {
315351
});
316352
}
317353

318-
ge(other: T | Series<T>): Series<boolean> {
354+
ge(other: Scalar | Series<Scalar>): Series<boolean> {
319355
return this._cmpOp(other, (a, b) => {
320356
if (a === null || b === null) {
321357
return false;
@@ -546,28 +582,7 @@ export class Series<T extends Scalar = Scalar> {
546582
/** Return a new Series sorted by values. */
547583
sortValues(ascending = true, naPosition: "first" | "last" = "last"): Series<T> {
548584
const pairs = this._values.map((v, i) => ({ v, i }));
549-
pairs.sort((a, b) => {
550-
const aNull =
551-
a.v === null || a.v === undefined || (typeof a.v === "number" && Number.isNaN(a.v));
552-
const bNull =
553-
b.v === null || b.v === undefined || (typeof b.v === "number" && Number.isNaN(b.v));
554-
if (aNull && bNull) {
555-
return 0;
556-
}
557-
if (aNull) {
558-
return naPosition === "first" ? -1 : 1;
559-
}
560-
if (bNull) {
561-
return naPosition === "first" ? 1 : -1;
562-
}
563-
const av = a.v as number | string | boolean;
564-
const bv = b.v as number | string | boolean;
565-
if (av === bv) {
566-
return 0;
567-
}
568-
const cmp = av < bv ? -1 : 1;
569-
return ascending ? cmp : -cmp;
570-
});
585+
pairs.sort((a, b) => compareScalars(a.v, b.v, ascending, naPosition));
571586
return new Series<T>({
572587
data: pairs.map(({ v }) => v),
573588
index: this.index.take(pairs.map(({ i }) => i)),
@@ -639,8 +654,8 @@ export class Series<T extends Scalar = Scalar> {
639654
// ─── set operations ───────────────────────────────────────────────────────
640655

641656
/** True when `value` exists in this Series. */
642-
isin(values: readonly T[]): Series<boolean> {
643-
const set = new Set<T>(values);
657+
isin(values: readonly Scalar[]): Series<boolean> {
658+
const set = new Set<Scalar>(values);
644659
return new Series<boolean>({
645660
data: this._values.map((v) => set.has(v)),
646661
index: this.index,

src/index.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ export type {
2929
export { Index } from "./core/index.ts";
3030
export type { IndexOptions } from "./core/index.ts";
3131
export { RangeIndex } from "./core/index.ts";
32-
export { Dtype } from "./core/dtype.ts";
33-
export type { DtypeKind, ItemSize } from "./core/dtype.ts";
34-
export { Series } from "./core/series.ts";
35-
export type { SeriesOptions } from "./core/series.ts";
32+
export { Dtype } from "./core/index.ts";
33+
export type { DtypeKind, ItemSize } from "./core/index.ts";
34+
export { Series } from "./core/index.ts";
35+
export type { SeriesOptions } from "./core/index.ts";

tests/core/dtype.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
* Tests for the Dtype system.
33
*/
44
import { describe, expect, it } from "bun:test";
5-
import { Dtype } from "../../src/core/index.ts";
5+
import { Dtype } from "../../src/index.ts";
66

77
describe("Dtype", () => {
88
describe("singletons", () => {

tests/core/series.test.ts

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
* Tests for Series.
33
*/
44
import { describe, expect, it } from "bun:test";
5-
import { Dtype, Index, Series } from "../../src/core/index.ts";
5+
import { Dtype, Index, Series } from "../../src/index.ts";
66
describe("Series", () => {
77
describe("construction", () => {
88
it("creates from numeric array with default RangeIndex", () => {
@@ -99,30 +99,30 @@ describe("Series", () => {
9999
const a = new Series({ data: [1, 2, 3] });
100100
const b = new Series({ data: [4, 5, 6] });
101101

102-
it("add scalar", () => expect(a.add(10 as never).values).toEqual([11, 12, 13]));
103-
it("add series", () => expect(a.add(b as never).values).toEqual([5, 7, 9]));
104-
it("sub series", () => expect(b.sub(a as never).values).toEqual([3, 3, 3]));
105-
it("mul scalar", () => expect(a.mul(2 as never).values).toEqual([2, 4, 6]));
106-
it("div series", () => expect(b.div(a as never).values).toEqual([4, 2.5, 2]));
107-
it("mod", () => expect(b.mod(a as never).values).toEqual([0, 1, 0]));
108-
it("pow", () => expect(a.pow(2 as never).values).toEqual([1, 4, 9]));
109-
it("floordiv", () => expect(b.floordiv(a as never).values).toEqual([4, 2, 2]));
102+
it("add scalar", () => expect(a.add(10).values).toEqual([11, 12, 13]));
103+
it("add series", () => expect(a.add(b).values).toEqual([5, 7, 9]));
104+
it("sub series", () => expect(b.sub(a).values).toEqual([3, 3, 3]));
105+
it("mul scalar", () => expect(a.mul(2).values).toEqual([2, 4, 6]));
106+
it("div series", () => expect(b.div(a).values).toEqual([4, 2.5, 2]));
107+
it("mod", () => expect(b.mod(a).values).toEqual([0, 1, 0]));
108+
it("pow", () => expect(a.pow(2).values).toEqual([1, 4, 9]));
109+
it("floordiv", () => expect(b.floordiv(a).values).toEqual([4, 2, 2]));
110110

111111
it("throws when sizes differ", () => {
112112
const c = new Series({ data: [1, 2] });
113-
expect(() => a.add(c as never)).toThrow(RangeError);
113+
expect(() => a.add(c)).toThrow(RangeError);
114114
});
115115
});
116116

117117
describe("comparison", () => {
118118
const s = new Series({ data: [1, 2, 3] });
119119

120-
it("eq", () => expect(s.eq(2 as never).values).toEqual([false, true, false]));
121-
it("ne", () => expect(s.ne(2 as never).values).toEqual([true, false, true]));
122-
it("lt", () => expect(s.lt(2 as never).values).toEqual([true, false, false]));
123-
it("le", () => expect(s.le(2 as never).values).toEqual([true, true, false]));
124-
it("gt", () => expect(s.gt(2 as never).values).toEqual([false, false, true]));
125-
it("ge", () => expect(s.ge(2 as never).values).toEqual([false, true, true]));
120+
it("eq", () => expect(s.eq(2).values).toEqual([false, true, false]));
121+
it("ne", () => expect(s.ne(2).values).toEqual([true, false, true]));
122+
it("lt", () => expect(s.lt(2).values).toEqual([true, false, false]));
123+
it("le", () => expect(s.le(2).values).toEqual([true, true, false]));
124+
it("gt", () => expect(s.gt(2).values).toEqual([false, false, true]));
125+
it("ge", () => expect(s.ge(2).values).toEqual([false, true, true]));
126126
});
127127

128128
describe("filter", () => {
@@ -134,7 +134,7 @@ describe("Series", () => {
134134
});
135135

136136
it("filters by boolean Series", () => {
137-
const mask = s.gt(2 as never);
137+
const mask = s.gt(2);
138138
const result = s.filter(mask);
139139
expect(result.values).toEqual([3, 4, 5]);
140140
});
@@ -173,7 +173,7 @@ describe("Series", () => {
173173
it("nunique", () => expect(s.nunique()).toBe(5));
174174

175175
it("sum with nulls skips them", () => {
176-
const s2 = new Series({ data: [1, null, 2, null, 3] as never[] });
176+
const s2 = new Series({ data: [1, null, 2, null, 3] });
177177
expect(s2.sum()).toBe(6);
178178
});
179179

@@ -245,7 +245,7 @@ describe("Series", () => {
245245
describe("isin", () => {
246246
it("returns boolean mask", () => {
247247
const s = new Series({ data: [1, 2, 3, 4] });
248-
expect(s.isin([2, 4] as never[]).values).toEqual([false, true, false, true]);
248+
expect(s.isin([2, 4]).values).toEqual([false, true, false, true]);
249249
});
250250
});
251251

0 commit comments

Comments
 (0)