Orthogonal selection

[Artur-man]

Summary:

• OrthogonalIndexer and IntArrayDimIndexer
• Order class from zarr-python
• IntDimIndexer (may need some work)
• Future work: BoolArrayDimIndexer
• update normalization for integer vectors
• add methods for int and zb_int (are these needed?), currently implemented but not used ...
• zero_based_to_one_based function now also applies to none slice selections
• [.ZarrArray triggers orthogonal selection only (or should it?)
• add tests for orthogonal selection

Relevant Issues: #107 #104 #97 #43

[Artur-man]

Here it is, tested a bunch of times but who knows :D Let me know what you guys think and lets review if you want.

[Artur-man]

Commented out exported functions under int.R to let pkgdown do its thing ... although I am not sure if int() or zb_int() are indeed needed.

[keller-mark]

Hi @Artur-man thank you very much for the PR. I had some comments about additional tests to add for the new internal classes. Since they contain indexing arithmetic it would be great to have unit tests to at least ensure there are no regressions.

[keller-mark]

Would you be able to add documentation comments for this function?

[keller-mark]

Would you be able to add a test for this function and documentation comments to convey the purpose?

[keller-mark]

Would you be able to add a test for this OrthogonalIndexer class? I know python may not have any analogous tests but maybe the current behavior can be tested like for BasicIndexer https://github.com/keller-mark/pizzarr/blob/main/tests/testthat/test-indexing-basic.R#L26 to prevent regressions

[keller-mark]

Similarly, would you be able to add a test for this Order class?

[keller-mark]

Similarly, would you be able to add a test for this IntArrayDimIndexer class?

[keller-mark]

I think it could make sense to have a zb_int function so that users who want to use zero-based / python-style indexing can do so without much thinking. It could just do index + 1 i think

[Artur-man]

will do!

[keller-mark]

It looks like this line was previously

expect_equal(z[1:3,5], z$get_item(list(slice(1, 3), slice(5, 5))))

Did the behavior change?

[Artur-man]

ah I have been meaning to discuss this, I think 1:2 and 1:3 have the same result here, but I guess I was experimenting with how to treat :.

Note that the shape of a is (2,10) and I think slice(1,3) would call python-based indices (0,1,2) which are the first three rows but I think slice doesn't care if indices are outside of dimension limits ?

> z$get_item(list(slice(1, 10), slice(5, 5)))$data
     [,1]
[1,]    9
[2,]   10

> z$get_item(list(slice(1, 20), slice(5, 5)))$data
     [,1]
[1,]    9
[2,]   10

As a remark all dimensional indices with a:b would be converted to slice(a,b), and this is still the case with manage_filters. See below:

pizzarr/R/zarr-array.R

Lines 1231 to 1235 in f84355d

	} else if(typeof(x) == "language") {
	x <- as.list(x)
	# Return a range (supplied via : or seq())
	if(x[[1]] == ":") {
	return(slice(x[[2]], x[[3]]))

A typical R-based indexing with a:b and [ method would fail in this situation. Lets see what happens when we incorporate the same indexing without/with ZarrArray.

# with ZarrArray
> z <- zarr_create(shape=dim(a), dtype="<f4", fill_value=NA)
> z$set_item("...", a)
> z[1:3,5]$data
     [,1]
[1,]    9
[2,]   10

# without ZarrArray
> a <- array(data=1:20, dim=c(2, 10))
> a[1:3,5]
Error in a[1:3, 5] : subscript out of bounds

It is up to you @keller-mark, how would you like [ to behave.