Skip to content

Commit 2238fce

Browse files
Copilotmrjf
andauthored
Fix 15 bugs across cut, digitize, DataFrame, insertColumn, strExtractGroups, linspace, and tests
Source fixes: - cut_qcut: right=false edge computation, degenerate step=0, float-point drift on last edge - numeric_extended: digitize right=true bin assignment off-by-one - numeric_extended: linspace preserves -0 start endpoint - frame: constructor accepts optional columnNames for duplicate column support - frame: shape/size/empty use columns.size (respects duplicate names) - insert_pop: insertColumn builds column names array for allowDuplicates - string_ops_extended: strExtractGroups handles all-non-matching rows via group counting Test fixes (clear assertion/logic bugs): - zscore ddof=0: range0 > range1 (population std is smaller, not larger) - rolling_apply custom min: min([1,5,9])=1 not 5 - rolling_apply count: 1 valid value in [null,3,null] not 2 - window_extended rollingSkew: [1,9,10] is left-skewed, not [10,2,1] - window_extended rollingQuantile q=0: min([1,5,9])=1 not 5 - wide_to_long missing stub: use multi-stub fixture so suffix "2" is discoverable - wide_to_long property: account for duplicate id values in assertion - to_from_dict split round-trip: skip when colB shorter than colA - strDedent property: filter lines to have non-whitespace content Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: mrjf <180956+mrjf@users.noreply.github.com>
1 parent eff0a57 commit 2238fce

11 files changed

Lines changed: 88 additions & 28 deletions

src/core/frame.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -102,10 +102,14 @@ export class DataFrame {
102102
* @param columns - Ordered map of column name → Series (all same length and index).
103103
* @param index - Row index (must match each Series' length).
104104
*/
105-
constructor(columns: ReadonlyMap<string, Series<Scalar>>, index: Index<Label>) {
105+
constructor(
106+
columns: ReadonlyMap<string, Series<Scalar>>,
107+
index: Index<Label>,
108+
columnNames?: readonly string[],
109+
) {
106110
this._columns = columns;
107111
this.index = index;
108-
this.columns = new Index<string>([...columns.keys()]);
112+
this.columns = new Index<string>(columnNames ?? [...columns.keys()]);
109113
}
110114

111115
/**
@@ -208,7 +212,7 @@ export class DataFrame {
208212

209213
/** `[nRows, nCols]` — mirrors `pandas.DataFrame.shape`. */
210214
get shape(): [number, number] {
211-
return [this.index.size, this._columns.size];
215+
return [this.index.size, this.columns.size];
212216
}
213217

214218
/** Always `2`. */
@@ -218,12 +222,12 @@ export class DataFrame {
218222

219223
/** Total number of cells (`nRows * nCols`). */
220224
get size(): number {
221-
return this.index.size * this._columns.size;
225+
return this.index.size * this.columns.size;
222226
}
223227

224228
/** `true` when the DataFrame has no rows or no columns. */
225229
get empty(): boolean {
226-
return this.index.size === 0 || this._columns.size === 0;
230+
return this.index.size === 0 || this.columns.size === 0;
227231
}
228232

229233
// ─── column access ────────────────────────────────────────────────────────

src/core/insert_pop.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,23 +82,28 @@ export function insertColumn(
8282
}
8383

8484
// Rebuild the column map, inserting the new column at position `loc`.
85+
const colNames: string[] = [];
8586
const colMap = new Map<string, Series<Scalar>>();
8687
let idx = 0;
8788

8889
for (const colName of df.columns.values) {
8990
if (idx === loc) {
90-
colMap.set(column, series);
91+
colNames.push(column);
9192
}
93+
colNames.push(colName);
9294
colMap.set(colName, df.col(colName));
9395
idx++;
9496
}
9597

9698
// Handle insertion at the end (loc === nCols).
9799
if (loc === nCols) {
98-
colMap.set(column, series);
100+
colNames.push(column);
99101
}
100102

101-
return new DataFrame(colMap, df.index);
103+
// Always add the new column data to the map (last-wins for duplicate names).
104+
colMap.set(column, series);
105+
106+
return new DataFrame(colMap, df.index, colNames);
102107
}
103108

104109
// ─── popColumn ────────────────────────────────────────────────────────────────

src/stats/cut_qcut.ts

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -260,11 +260,27 @@ export function cut(
260260
if (mn === mx) {
261261
throw new Error("Cannot cut constant array (all values identical).");
262262
}
263-
const step = (mx - mn) / bins;
264-
edges = Array.from({ length: bins + 1 }, (_, i) => mn + i * step);
265-
// Slightly extend the lower edge so the minimum value is included
266-
edges[0] = mn - step * 0.001;
267-
edges = deduplicateEdges(edges, duplicates);
263+
if (right) {
264+
const step = (mx - mn) / bins;
265+
edges = Array.from({ length: bins + 1 }, (_, i) => mn + i * step);
266+
// Extend the lower edge so the minimum is inside the first (a, b] interval
267+
edges[0] = mn - step * 0.001;
268+
// Pin the upper edge to the exact max (avoids floating-point drift)
269+
edges[edges.length - 1] = mx;
270+
} else {
271+
// For left-closed [a, b) intervals, extend the upper end so the max is included;
272+
// linspace from mn to adjustedMx distributes the extension across all edges,
273+
// keeping boundary values in the lower bin (matching pandas behaviour).
274+
const adjustedMx = mx + (mx - mn) * 0.001;
275+
const step = (adjustedMx - mn) / bins;
276+
edges = Array.from({ length: bins + 1 }, (_, i) => mn + i * step);
277+
}
278+
// Auto-computed edges may have floating-point duplicates for tiny ranges;
279+
// silently drop them to avoid spurious errors.
280+
edges = deduplicateEdges(edges, "drop");
281+
if (edges.length < 2) {
282+
throw new Error("Cannot cut constant array (all values identical).");
283+
}
268284
} else {
269285
if (bins.length < 2) {
270286
throw new Error("At least 2 bin edges must be supplied.");

src/stats/numeric_extended.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -144,9 +144,12 @@ export function digitize(
144144
const n = bins.length;
145145
if (right) {
146146
// open left, closed right: bins[i-1] < v <= bins[i]
147+
if (v < (bins[0] as number)) {
148+
return -1;
149+
}
147150
for (let i = 0; i < n; i++) {
148151
if (v <= (bins[i] as number)) {
149-
return i - 1; // below first edge → -1
152+
return i;
150153
}
151154
}
152155
return n - 1; // above last edge
@@ -299,7 +302,13 @@ export function linspace(start: number, stop: number, num = 50): number[] {
299302
const step = (stop - start) / (num - 1);
300303
const result: number[] = [];
301304
for (let i = 0; i < num; i++) {
302-
result.push(i === num - 1 ? stop : start + i * step);
305+
if (i === num - 1) {
306+
result.push(stop);
307+
} else if (i === 0) {
308+
result.push(start);
309+
} else {
310+
result.push(start + i * step);
311+
}
303312
}
304313
return result;
305314
}

src/stats/string_ops_extended.ts

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,11 @@ export function strExtractGroups(
184184
});
185185
});
186186

187-
const width = rows.reduce((w, r) => Math.max(w, r.length), 0);
187+
const numGroups = countCaptureGroups(re);
188+
const width = Math.max(
189+
rows.reduce((w, r) => Math.max(w, r.length), 0),
190+
numGroups,
191+
);
188192

189193
// Use named groups if available and count matches; otherwise use 0-indexed strings.
190194
const colNames: string[] =
@@ -205,6 +209,21 @@ export function strExtractGroups(
205209
return DataFrame.fromColumns(columns, { index: rowIndex(input) });
206210
}
207211

212+
/** Count the number of capture groups in a regex (excluding non-capturing groups). */
213+
function countCaptureGroups(re: RegExp): number {
214+
let count = 0;
215+
const src = re.source;
216+
for (let i = 0; i < src.length; i++) {
217+
const ch = src[i];
218+
if (ch === "\\") {
219+
i++; // skip escaped char
220+
} else if (ch === "(" && src[i + 1] !== "?") {
221+
count++;
222+
}
223+
}
224+
return count;
225+
}
226+
208227
/** Parse named capture group names from a regex source string. */
209228
function extractGroupNames(re: RegExp): string[] {
210229
const namedGroupPattern = /\(\?<([^>]+)>/g;

tests/core/to_from_dict.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,7 @@ describe("property-based", () => {
259259
fc.array(fc.integer({ min: 1, max: 10 }), { minLength: 1, maxLength: 4 }),
260260
fc.array(fc.integer({ min: 1, max: 10 }), { minLength: 1, maxLength: 4 }),
261261
(colA, colB) => {
262+
if (colB.length < colA.length) return true;
262263
const df = DataFrame.fromColumns({ a: colA, b: colB.slice(0, colA.length) });
263264
const split = toDictOriented(df, "split");
264265
const df2 = fromDictOriented(split, "split");

tests/reshape/wide_to_long.test.ts

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -134,10 +134,10 @@ describe("wideToLong — multiple id columns", () => {
134134

135135
describe("wideToLong — missing stub columns", () => {
136136
test("missing wide column fills with null", () => {
137-
// A1 exists but A2 does not — A2 values should be null
138-
const df = DataFrame.fromColumns({ id: [1], A1: [10] });
139-
const long = wideToLong(df, "A", "id", "n", { suffix: "[12]" });
140-
// suffix 1 → A1=10, suffix 2 → A2=null
137+
// A1 and B1/B2 exist; A2 does not — A2 values should be null
138+
const df = DataFrame.fromColumns({ id: [1], A1: [10], B1: [20], B2: [30] });
139+
const long = wideToLong(df, ["A", "B"], "id", "n", { suffix: "[12]" });
140+
// suffix "2" discovered from B2; A2 missing → null
141141
expect(long.col("A").values).toEqual([10, null]);
142142
});
143143
});
@@ -200,8 +200,14 @@ describe("property-based", () => {
200200
const df = DataFrame.fromColumns(colData);
201201
const long = wideToLong(df, "v", "id", "n");
202202
const outId = long.col("id").values;
203-
// Each original id value should appear nSuffix times
204-
return idVals.every((v) => outId.filter((x) => x === v).length === nSuffix);
203+
// Total row count should be originalRows × nSuffix
204+
if (outId.length !== idVals.length * nSuffix) return false;
205+
// Each unique id value appears (its original count × nSuffix) times
206+
const uniqueIds = [...new Set(idVals)];
207+
return uniqueIds.every((v) => {
208+
const origCount = idVals.filter((x) => x === v).length;
209+
return outId.filter((x) => x === v).length === origCount * nSuffix;
210+
});
205211
},
206212
),
207213
);

tests/stats/numeric_extended.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,7 @@ describe("zscore", () => {
312312
// ddof=0 produces smaller values (divided by larger std denominator)
313313
const range0 = Math.max(...z0) - Math.min(...z0);
314314
const range1 = Math.max(...z1) - Math.min(...z1);
315-
expect(range0).toBeLessThan(range1);
315+
expect(range0).toBeGreaterThan(range1);
316316
});
317317

318318
it("preserves index labels", () => {

tests/stats/string_ops_extended.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -411,7 +411,7 @@ describe("strDedent — property tests", () => {
411411
fc.assert(
412412
fc.property(
413413
fc.array(
414-
fc.string({ minLength: 1 }).filter((s) => !s.includes("\n")),
414+
fc.string({ minLength: 1 }).filter((s) => !s.includes("\n") && s.trim().length > 0 && s.trimStart() === s),
415415
{
416416
minLength: 1,
417417
maxLength: 5,

tests/stats/window_extended.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ describe("rollingSkew", () => {
133133
});
134134

135135
it("left-skewed data → negative skew", () => {
136-
const out = vals(rollingSkew(s([10, 2, 1]), 3));
136+
const out = vals(rollingSkew(s([1, 9, 10]), 3));
137137
const v = out[2];
138138
expect(typeof v).toBe("number");
139139
expect((v as number) < 0).toBe(true);
@@ -221,7 +221,7 @@ describe("rollingQuantile", () => {
221221
expect(close(out[2] as number, 1)).toBe(true);
222222
expect(close(out[3] as number, 1)).toBe(true);
223223
expect(close(out[4] as number, 1)).toBe(true);
224-
expect(close(out[5] as number, 5)).toBe(true);
224+
expect(close(out[5] as number, 1)).toBe(true);
225225
});
226226

227227
it("q=1 is rolling maximum", () => {

0 commit comments

Comments
 (0)