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

clarify restrictions on axes for general interface #29062

Closed
tpapp opened this issue Sep 6, 2018 · 1 comment · Fixed by #37852
Closed

clarify restrictions on axes for general interface #29062

tpapp opened this issue Sep 6, 2018 · 1 comment · Fixed by #37852
Labels
arrays [a, r, r, a, y, s] docs This change adds or pertains to documentation

Comments

@tpapp
Copy link
Contributor

tpapp commented Sep 6, 2018

I am opening the issue following this discussion about the interface for general indexing.

The manual on Interfaces and custom indices specifies that axes should return (tuples of) AbstractUnitRange{T}.

In principle this would allow any T for which oneunit is defined, eg Dates or a wrapper type for floats (I have an example in the discussion I link above). This makes it tricky when one wants to perform arithmetic on indexes, in cases where a simple loop is not sufficient.

OTOH, if the implicit assumption is that T <: Integer, as @mbauman suggested in the discussion, it would be great if the manual could clarify that.

@tpapp
Copy link
Contributor Author

tpapp commented Sep 21, 2020

Another clarification that could be added to the docs of axes is that axes themselves could use generalized indexing.

This is currently the case with OffsetArrays.IdOffsetRange, eg the following code in general will not work:

function all_same(v::AbstractVector)
    v1 = first(v)
    for i in axes(v, 1)[2:end] # WRONG: use (begin+1):end
        v[i] == v1 || return false
    end
    true
end

but this may trap an unwary user.

@timholy, what do you think?

@StefanKarpinski StefanKarpinski added arrays [a, r, r, a, y, s] docs This change adds or pertains to documentation labels Sep 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrays [a, r, r, a, y, s] docs This change adds or pertains to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants