Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow getindex(::MyType, ::Colon) to be customized #9419

Closed
rfourquet opened this issue Dec 20, 2014 · 5 comments · Fixed by #10331
Closed

allow getindex(::MyType, ::Colon) to be customized #9419

rfourquet opened this issue Dec 20, 2014 · 5 comments · Fixed by #10331

Comments

@rfourquet
Copy link
Member

And similarly for setindex!.
For example:

type A end
Base.getindex(a::A, ::Range) = 0
Base.getindex(a::A, ::Colon) = 1

julia> A()[:]
ERROR: `endof` has no method matching endof(::A)

This is worse if endof(::A) is defined, as then A()[:] chooses silently the unintended getindex method.

If allowing customization of Base.getindex(a::A, ::Colon) does not happen, I would request that a[:] is translated to a[beginof(a):endof(a)], as some indexable collections have an indice starting at something else than 1.

At the very least, the doc for getindex/setindex! should mention this special case, where "The syntax a[i,j,...] is converted by the compiler to getindex(a, i, j, ...)" does not seem true.

Also, in "base/subarray.jl", this method is defined: getindex{T}(v::AbstractArray{T,1}, ::Colon) = v, should it be removed or is it used?

@timholy
Copy link
Member

timholy commented Dec 20, 2014

This is something that needs to be changed in the parser, and is part of #9150.

Re the getindex method in base/subarray.jl, I suspect it's there currently just to resolve a method ambiguity. But someday it will be directly useful.

@mlubin
Copy link
Member

mlubin commented Dec 22, 2014

👍
This is essential for proper slicing semantics of non-one-based arrays that are commonly used in JuMP.

@jakebolewski
Copy link
Member

I have a branch that already does this, has @andreasnoack not included it in #9150?

@andreasnoack
Copy link
Member

Yes. It is part of #9150 which I'll hopefully will finalize between Christmas and the end of the year.

@rfourquet
Copy link
Member Author

Oh I had seen #9150 before posting, but didn't understand it did hat I wanted, good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants