JuliaLang / Juleps

Julia Enhancement Proposals
Other
67 stars 24 forks source link

Describe problem with sentinel values for non-1 arrays #27

Closed timholy closed 7 years ago

timholy commented 7 years ago

CC @nalimilan

nalimilan commented 7 years ago

Thanks. I like the discussion, but the Julep explicitly lists the issue of non-standard indices in the "out of scope" section. So either add the discussion there, or make a concrete suggestion which fits with the two general proposals and remove that mention.

That may not be as hard as it sounds: we can just decide that by default all functions return 1-based linear indices, but that they will all accept a type as first parameter to get a different kind of indices. The question is, should we keep returning 1-based linear indices by default, or n-based linear indices (with arbitrary n depending on the array type)?

timholy commented 7 years ago

we can just decide that by default all functions return 1-based linear indices

This runs into a problem for AbstractVectors, where it's hard to tell whether v[i] is supposed to be a 1-based linear index or the cartesian index. We have decided on the latter meaning; we might have to introduce a LinearIndex type if we want to be able to have it mean the former.

For any other dimensionality, it's fine.

But sure, I will change it more comprehensively so that it's not "out of scope." It's such a nice Julep for discussing this troublesome family of functions, and I think it's worth laying out all the issues on the table rather than having a separate Julep just for this one issue.

timholy commented 7 years ago

Wait, I don't think you do treat non-1 indexing; there's a section on linear vs cartesian indexes, but that's not the same thing. Can you point me to what you mean?

nalimilan commented 7 years ago

This runs into a problem for AbstractVectors, where it's hard to tell whether v[i] is supposed to be a 1-based linear index or the cartesian index. We have decided on the latter meaning; we might have to introduce a LinearIndex type if we want to be able to have it mean the former.

I'm not sure I follow. Could you precise what you mean? I just suggested we do the same as for enumerate, i.e. return the 1-based position of the found entry by default, and return other kinds of indices when passing an index type as first argument.

Wait, I don't think you do treat non-1 indexing; there's a section on linear vs cartesian indexes, but that's not the same thing. Can you point me to what you mean?

Yes, I referred to that section. It doesn't deal with non-1 indices, but that's really the same issue from an API perspective AFAICT.

timholy commented 7 years ago

I'm not sure I follow. Could you precise what you mean? I just suggested we do the same as for enumerate, i.e. return the 1-based position of the found entry by default, and return other kinds of indices when passing an index type as first argument.

We've followed a firm rule that A[CartesianIndex((i,j))] does exactly the same thing as A[i,j]. For 1-d arrays, this means we can't easily distinguish cartesian indexing from linear indexing. For example:

julia> x = OffsetArray(1:9, -4:4)
OffsetArrays.OffsetArray{Int64,1,UnitRange{Int64}} with indices -4:4:
 1
 2
 3
 4
 5
 6
 7
 8
 9

julia> x[1]
6

Currently we don't have a LinearIndex type that would let 1 always be interpreted as the first index, no matter what. Relevant discussion about a potential way to unify these, and some of the negatives: https://github.com/JuliaLang/julia/pull/20573#issuecomment-279226351.

Does this help? I think that also addresses the second point as to why I see these as quite separate issues.

nalimilan commented 7 years ago

I see, thanks. So we need to decide what's the best default behavior; I think it would make sense to behave like enumerate for consistency and use 1-based indexing. Then we can discuss what IndexLinear/LinearIndex would mean when we add it.

timholy commented 7 years ago

I'll let you decide about merging this, I just didn't want this issue to be forgotten as folks think about the redesign.

nalimilan commented 7 years ago

If you don't have the time to add a full proposal to fix this, could you just insert the paragraph in the "out of scope" section for now? As I said, the list where you added it currently only contains questions which are addressed by the two proposals, which isn't the case of this one.

timholy commented 7 years ago

I'm slow, but now I get it :smile:.