Skip to content

Commit 42eaa70

Browse files
manztmanzt
authored
feat(indexing/ndarray)!: Remove .indices from Slice interface (#121)
1 parent 6017e13 commit 42eaa70

File tree

9 files changed

+57
-58
lines changed

9 files changed

+57
-58
lines changed

‎.changeset/slimy-pigs-buy.md

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
"@zarrita/indexing": minor
3+
"@zarrita/ndarray": minor
4+
---
5+
6+
feat: Remove `.indices` methods from `Slice` interface
7+
8+
By making `Slice` a more minimal interface, it is easy to allow compatability with ZarrJS `slice`
9+
(or slices not generated by the `slice` helper function).

‎packages/indexing/__tests__/setter.test.ts

+10-6
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import type { Chunk, DataType } from "@zarrita/core";
55

66
import type { Projection } from "../src/types.js";
77
import { setter } from "../src/ops.js";
8-
import { slice } from "../src/util.js";
8+
import { slice, slice_indices } from "../src/util.js";
99

1010
/** Compute strides for 'C' or 'F' ordered array from shape */
1111
function get_strides(shape: readonly number[], order: "C" | "F") {
@@ -39,6 +39,10 @@ function to_c<D extends DataType>({ data, shape, stride }: Chunk<D>): Chunk<D> {
3939
return out;
4040
}
4141

42+
function indices(size: number) {
43+
return slice_indices(slice(null), size);
44+
}
45+
4246
describe("setter", () => {
4347
it("setter.set_scalar - fill", async () => {
4448
let a = setter.prepare(
@@ -47,7 +51,7 @@ describe("setter", () => {
4751
get_strides([2, 3, 4], "C"),
4852
);
4953

50-
let sel = [2, 3, 4].map((size) => slice(null).indices(size));
54+
let sel = [2, 3, 4].map(indices);
5155
setter.set_scalar(a, sel, 1);
5256
// deno-fmt-ignore
5357
expect(a.data).toStrictEqual(new Float32Array([
@@ -124,7 +128,7 @@ describe("setter", () => {
124128
get_strides([2, 3, 4], "C"),
125129
);
126130

127-
let sel = [slice(null).indices(2), slice(2).indices(3), 0];
131+
let sel = [indices(2), slice_indices(slice(2), 3), 0];
128132
setter.set_scalar(a, sel, 1);
129133
// deno-fmt-ignore
130134
expect(a.data).toStrictEqual(new Float32Array([
@@ -137,7 +141,7 @@ describe("setter", () => {
137141
0, 0, 0, 0,
138142
]));
139143

140-
sel = [0, slice(null).indices(3), slice(null).indices(4)];
144+
sel = [0, indices(3), indices(4)];
141145
setter.set_scalar(a, sel, 2);
142146

143147
// deno-fmt-ignore
@@ -159,7 +163,7 @@ describe("setter", () => {
159163
get_strides([2, 3, 4], "F"),
160164
);
161165

162-
let sel = [slice(null).indices(2), slice(2).indices(3), 0];
166+
let sel = [slice_indices(slice(null), 2), slice_indices(slice(2), 3), 0];
163167
setter.set_scalar(f, sel, 1);
164168
// deno-fmt-ignore
165169
expect(f.data).toStrictEqual(new Float32Array([
@@ -169,7 +173,7 @@ describe("setter", () => {
169173
0, 0, 0, 0, 0, 0,
170174
]));
171175

172-
sel = [0, slice(null).indices(3), slice(null).indices(4)];
176+
sel = [0, slice_indices(slice(null), 3), slice_indices(slice(null), 4)];
173177
setter.set_scalar(f, sel, 2);
174178

175179
// deno-fmt-ignore

‎packages/indexing/__tests__/slice.test.ts

+3-15
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ import * as ops from "../src/ops.js";
55
import { slice } from "../src/util.js";
66
import { IndexError } from "../src/indexer.js";
77

8-
// @deno-fmt-ignore
98
const DATA = {
9+
// @deno-fmt-ignore
1010
data: new Int32Array([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23 ]),
1111
shape: [2, 3, 4],
1212
stride: [12, 4, 1],
@@ -70,20 +70,8 @@ export function run_suite(name: string, getter: any) {
7070
[
7171
[1, null, null],
7272
{
73-
data: new Int32Array([
74-
12,
75-
13,
76-
14,
77-
15,
78-
16,
79-
17,
80-
18,
81-
19,
82-
20,
83-
21,
84-
22,
85-
23,
86-
]),
73+
// @deno-fmt-ignore
74+
data: new Int32Array([12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23]),
8775
shape: [3, 4],
8876
stride: [4, 1],
8977
},

‎packages/indexing/__tests__/util.test.ts

+20-20
Original file line numberDiff line numberDiff line change
@@ -1,58 +1,55 @@
11
import { describe, expect, test } from "vitest";
2-
import { range, slice } from "../src/util.js";
2+
import { range, slice, slice_indices } from "../src/util.js";
33

44
describe("slice", () => {
55
test("slice(null)", () => {
66
expect(slice(null)).toMatchInlineSnapshot(`
77
{
8-
"indices": [Function],
98
"start": null,
109
"step": null,
1110
"stop": null,
1211
}
1312
`);
1413
});
1514

16-
test("slice(null).indices(10)", () => {
17-
expect(slice(null).indices(10)).toStrictEqual([0, 10, 1]);
15+
test("slice_indices(slice(null), 10)", () => {
16+
expect(slice_indices(slice(null), 10)).toStrictEqual([0, 10, 1]);
1817
});
1918

2019
test("slice(3, 15, 2)", () => {
2120
expect(slice(3, 15, 2)).toMatchInlineSnapshot(`
2221
{
23-
"indices": [Function],
2422
"start": 3,
2523
"step": 2,
2624
"stop": 15,
2725
}
2826
`);
2927
});
3028

31-
test("slice(3, 15, 2).indices(10)", () => {
32-
expect(slice(3, 15, 2).indices(10)).toStrictEqual([3, 10, 2]);
29+
test("slice_indices(slice(3, 15, 2), 10)", () => {
30+
expect(slice_indices(slice(3, 15, 2), 10)).toStrictEqual([3, 10, 2]);
3331
});
3432

35-
test("slice(3, 15, 2).indices(30)", () => {
36-
expect(slice(3, 15, 2).indices(30)).toStrictEqual([3, 15, 2]);
33+
test("slice_indices(slice(3, 15, 2), 30)", () => {
34+
expect(slice_indices(slice(3, 15, 2), 30)).toStrictEqual([3, 15, 2]);
3735
});
3836

3937
test("slice(40)", () => {
4038
expect(slice(40)).toMatchInlineSnapshot(`
4139
{
42-
"indices": [Function],
4340
"start": null,
4441
"step": null,
4542
"stop": 40,
4643
}
4744
`);
4845
});
4946

50-
test("slice(40).indices(10)", () => {
51-
expect(slice(40).indices(4)).toStrictEqual([0, 4, 1]);
47+
test("slice_indices(slice(40), 4)", () => {
48+
expect(slice_indices(slice(40), 4)).toStrictEqual([0, 4, 1]);
5249
});
5350

54-
test("slice(40).indices(41)", () => {
55-
expect(slice(40).indices(41)).toStrictEqual([0, 40, 1]);
51+
test("slice_indices(slice(40), 41)", () => {
52+
expect(slice_indices(slice(40), 41)).toStrictEqual([0, 40, 1]);
5653
});
5754
});
5855

@@ -61,8 +58,8 @@ describe("slice indices", () => {
6158
[null, 10, [0, 10, 1]],
6259
[40, 4, [0, 4, 1]],
6360
[40, 41, [0, 40, 1]],
64-
])("slice(%o).indices(%i) -> %o", (arg, indices, expected) => {
65-
expect(slice(arg).indices(indices)).toStrictEqual(expected);
61+
])("slice_indices(slice(%o), %i) -> %o", (arg, indices, expected) => {
62+
expect(slice_indices(slice(arg), indices)).toStrictEqual(expected);
6663
});
6764

6865
test.each([
@@ -73,18 +70,21 @@ describe("slice indices", () => {
7370
[null, null, -3, 14, [13, -1, -3]],
7471
[null, null, -3, 2, [1, -1, -3]],
7572
])(
76-
"slice(%o, %o, %o).indices(%i) -> %o",
73+
"slice_indices(slice(%o, %o, %o), %i) -> %o",
7774
(start, stop, step, indices, expected) => {
78-
expect(slice(start, stop, step).indices(indices)).toStrictEqual(expected);
75+
expect(slice_indices(slice(start, stop, step), indices)).toStrictEqual(
76+
expected,
77+
);
7978
},
8079
);
8180

8281
test.each([
8382
[null, null, 0, 1],
8483
])(
85-
`slice(%o, %o, %o).indices(%i) -> throws`,
84+
`slice_indices(slice(%o, %o, %o), %i) -> throws`,
8685
(start, stop, step, indices) => {
87-
expect(() => slice(start, stop, step).indices(indices)).toThrowError();
86+
expect(() => slice_indices(slice(start, stop, step), indices))
87+
.toThrowError();
8888
},
8989
);
9090
});

‎packages/indexing/src/index.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
export { get, set } from "./ops.js";
22
export { get as get_with_setter } from "./get.js";
33
export { set as set_with_setter } from "./set.js";
4-
export { slice } from "./util.js";
4+
export { slice, slice_indices as _slice_indices } from "./util.js";
55
export type * from "./types.js";

‎packages/indexing/src/indexer.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { Indices, Slice } from "./types.js";
2-
import { product, range, slice } from "./util.js";
2+
import { product, range, slice, slice_indices } from "./util.js";
33

44
export class IndexError extends Error {
55
constructor(msg: string) {
@@ -109,7 +109,7 @@ class SliceDimIndexer {
109109

110110
constructor({ dim_sel, dim_len, dim_chunk_len }: SliceDimIndexerProps) {
111111
// normalize
112-
const [start, stop, step] = dim_sel.indices(dim_len);
112+
const [start, stop, step] = slice_indices(dim_sel, dim_len);
113113
this.start = start;
114114
this.stop = stop;
115115
this.step = step;

‎packages/indexing/src/types.ts

-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ export interface Slice {
66
start: number | null;
77
stop: number | null;
88
step: number | null;
9-
indices: (length: number) => Indices;
109
}
1110

1211
export type Projection =

‎packages/indexing/src/util.ts

+2-7
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,8 @@ export function* product<T extends Array<Iterable<any>>>(
5151
}
5252

5353
// https://github.com/python/cpython/blob/263c0dd16017613c5ea2fbfc270be4de2b41b5ad/Objects/sliceobject.c#L376-L519
54-
function slice_indices(
55-
start: number | null,
56-
stop: number | null,
57-
step: number | null,
54+
export function slice_indices(
55+
{ start, stop, step }: Slice,
5856
length: number,
5957
): Indices {
6058
if (step === 0) {
@@ -117,9 +115,6 @@ export function slice(
117115
start,
118116
stop,
119117
step,
120-
indices(length: number) {
121-
return slice_indices(this.start, this.stop, this.step, length);
122-
},
123118
};
124119
}
125120

‎packages/ndarray/__tests__/setter.test.ts

+10-6
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,11 @@ import { describe, expect, it } from "vitest";
22
import ndarray, { type TypedArray } from "ndarray";
33
import { assign } from "ndarray-ops";
44

5-
import { type Projection, slice } from "@zarrita/indexing";
5+
import {
6+
_slice_indices as slice_indices,
7+
type Projection,
8+
slice,
9+
} from "@zarrita/indexing";
610

711
import { setter } from "../src/index.js";
812

@@ -48,7 +52,7 @@ describe("setter", () => {
4852
get_strides([2, 3, 4], "C"),
4953
);
5054

51-
let sel = [2, 3, 4].map((size) => slice(null).indices(size));
55+
let sel = [2, 3, 4].map((size) => slice_indices(slice(null), size));
5256
setter.set_scalar(a, sel, 1);
5357
// deno-fmt-ignore
5458
expect(a.data).toStrictEqual(new Float32Array([
@@ -125,7 +129,7 @@ describe("setter", () => {
125129
get_strides([2, 3, 4], "C"),
126130
);
127131

128-
let sel = [slice(null).indices(2), slice(2).indices(3), 0];
132+
let sel = [slice_indices(slice(null), 2), slice_indices(slice(2), 3), 0];
129133
setter.set_scalar(a, sel, 1);
130134
// deno-fmt-ignore
131135
expect(a.data).toStrictEqual(new Float32Array([
@@ -138,7 +142,7 @@ describe("setter", () => {
138142
0, 0, 0, 0,
139143
]));
140144

141-
sel = [0, slice(null).indices(3), slice(null).indices(4)];
145+
sel = [0, slice_indices(slice(null), 3), slice_indices(slice(null), 4)];
142146
setter.set_scalar(a, sel, 2);
143147

144148
// deno-fmt-ignore
@@ -160,7 +164,7 @@ describe("setter", () => {
160164
get_strides([2, 3, 4], "F"),
161165
);
162166

163-
let sel = [slice(null).indices(2), slice(2).indices(3), 0];
167+
let sel = [slice_indices(slice(null), 2), slice_indices(slice(2), 3), 0];
164168
setter.set_scalar(f, sel, 1);
165169
// deno-fmt-ignore
166170
expect(f.data).toStrictEqual(new Float32Array([
@@ -170,7 +174,7 @@ describe("setter", () => {
170174
0, 0, 0, 0, 0, 0,
171175
]));
172176

173-
sel = [0, slice(null).indices(3), slice(null).indices(4)];
177+
sel = [0, slice_indices(slice(null), 3), slice_indices(slice(null), 4)];
174178
setter.set_scalar(f, sel, 2);
175179

176180
// deno-fmt-ignore

0 commit comments

Comments
 (0)