Properly Generate ExtSlice

[vodik]

Multi-dimensional lookup should emit ast.ExtSlice instead of a normal lookup with ast.Tuple.

Basically any time we'd generate this:

slice=Index(value=Tuple(elts=[Num(n=3), Ellipsis(), Num(n=5)], ctx=Load())

But if any of the elements of that tuple is an instance of ast.slice (ast.Slice and, on Python 2, ast.Ellipsis), it should be constructed as slice=ExtSlice(dims[...]) instead.

Related to #1491 if we want to add Python 2 support for Ellipsis. Otherwise its not too pressing since we don't have anything that emits ast.Slice directly, its always done as a function call. But if we where to add something, this issue would have to be resolved first.

[Kodiologist]

Does this distinction matter at all in Python 3, or is it effectively a Python 2 issue?

[gilch]

Possibly related to #1481. If the distinction matters, Hy will have to have special forms for either case, which means . can't just be a macro.

[vodik]

@Kodiologist as far as I can tell, yes, but we get away with it because we never emit multidimensional slices anywhere with ast.Slice, always with a call to the slice constructor.

As far as I can tell, only cut emits ast.Slice, but its single dimension.

[vodik]

And for what its worth, its probably worth implementing - even if its an optimization when we notice slice construction inside indexing - because it performs much better:

In [1]: class Demo:
   ...:     def __getitem__(self, key):
   ...:         return key
   ...: 

In [2]: demo = Demo()

In [3]: %timeit demo[1:2]
183 ns ± 0.666 ns per loop (mean ± std. dev. of 7 runs, 10000000 loops each)

In [4]: %timeit demo[slice(1, 2)]
266 ns ± 2.23 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

[gilch]

On second thought, we could have a cuts special form for multiple dimensions and .* and implement . as a macro in terms of those. Or we could rethink cut. But how should the syntax look in Hy if we're not using slice?

[vodik]

I don't know how you could properly disambiguate it without using slice:

Say we chose to represent (cut foo 1 2) as (cut foo [1 2]), and then support taking in variable number of arguments. Well, problem is getitem, including at higher dimensions, isn't limited to just slices. It can take any arbitrary object. So the problem becomes determining if (cut foo [1 2]) is actually 1:2 or [1, 2].

[vodik]

I think I'll experiment with addressing this by trying to peek into contents of Index AST expression and start by trying to transforming (slice ...) into actual slice AST.

I don't think there's any real need to "fix" this by looking at how we approach representation, and instead look at it as a optimization instead. I think (get foo (slice 1 2)) is perfectly fine and readable, and probably preferable anyways. We could instead remove cut and replace it with a macro.

[thautwarm]

@vodik The reason why slice performs worse in your code is that global variable slice is loaded in each cycle.
In fact, slice could be slightly faster than literally getitem syntax.

class Demo:
    def __getitem__(self, item):
        return  item
    

demo = Demo()

s = slice(1, 2)
%timeit demo[1:2]
137 ns ± 0.564 ns per loop (mean ± std. dev. of 7 runs, 10000000 loops each)

%timeit demo[s]
130 ns ± 0.7 ns per loop (mean ± std. dev. of 7 runs, 10000000 loops each)

def test():
    s = slice(1, 2)
    demo = Demo()
    def test1():
        demo[1:2]
    def test2():
        demo[s]
    return test1, test2
test1, test2 = test()

%timeit test1()
203 ns ± 4.77 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

%timeit test2()
190 ns ± 4.65 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

[thautwarm]

More to fetch at serhiy-storchaka's PR.

[allison-casey]

Hy has dropped support for python2 and ExtSlice was removed in python3.9. Is there any reason this can't be closed since we still support 3.6-3.8?

[Kodiologist]

So to summarize, this seems to make neither a semantic difference nor a performance difference in practice, and it's going to be obsolete anyway as older Pythons die off. I think we probably don't need to worry about it.