Skip to content

Conversation

Tokazama
Copy link
Member

@Tokazama Tokazama commented Jun 17, 2022

This allows common comparison operators be parsed into indices (<, <=, >, >=). So instead of doing x[3:end] you can do x[>=(3)] (if x uses ArrayInterface.getindex. The main utility right now is that we can ensure that the indices are always inbounds using this syntax. I'm still working out the best way to communicate this info to checkbounds so that bounds checking is passed over (similar to how : becomes Slice which is known to be inbounds); but that will probably be more involved so I'm going to make that a separate effort.

@codecov
Copy link

codecov bot commented Jun 17, 2022

Codecov Report

Merging #316 (5ad45fd) into master (b922313) will increase coverage by 0.12%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #316      +/-   ##
==========================================
+ Coverage   91.74%   91.86%   +0.12%     
==========================================
  Files           9        9              
  Lines        1429     1439      +10     
==========================================
+ Hits         1311     1322      +11     
+ Misses        118      117       -1     
Impacted Files Coverage Δ
src/ArrayInterface.jl 86.17% <100.00%> (+0.22%) ⬆️
src/indexing.jl 89.75% <100.00%> (+0.77%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b922313...5ad45fd. Read the comment docs.

@Tokazama
Copy link
Member Author

@wangl-cc, you're welcome to review this since it may be of interest to you.

@wangl-cc
Copy link
Contributor

wangl-cc commented Jun 18, 2022

If checkbounds is needed, I think min or max can be removed, like the syntax

julia> x = 1:2
1:2

julia> Meta.@lower x[begin:3]
:($(Expr(:thunk, CodeInfo(
    @ none within `top-level scope`
1%1 = Base.firstindex(x)
│   %2 = %1:3%3 = Base.getindex(x, %2)
└──      return %3
))))

Besides, I think we can create some "functional range" like getindex(x, 3:last), which is much similar to the syntax x[3:end].

@Tokazama
Copy link
Member Author

The reason I used min and max was because those functions are returning all indices below or above the provided value. This is the same as if you filtered the axis with those functions. Maybe this needs to be clarified more in the docs.

We can't create a functional range like that because it would be type piracy.

@wangl-cc
Copy link
Contributor

wangl-cc commented Jun 18, 2022

The reason I used min and max was because those functions are returning all indices below or above the provided value. This is the same as if you filtered the axis with those functions. Maybe this needs to be clarified more in the docs.

With this interpretation, I think keeping them always inbounds is OK.

We can't create a functional range like that because it would be type piracy.

Maybe we can create new types as alternatives to begin and end to avoid type piracy.

@Tokazama
Copy link
Member Author

There is a package for the last suggestion, so it's probably more intuitive to do something similar or provide support through that package. I'll remove the firstindex and lastindex ones for now.

@Tokazama Tokazama merged commit 5228630 into master Jun 19, 2022
@Tokazama Tokazama deleted the to_index branch June 19, 2022 04:03
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 this pull request may close these issues.

2 participants