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

support CartesianIndices as axes for TileIterator #26

Open
johnnychen94 opened this issue Nov 6, 2020 · 2 comments · May be fixed by #27
Open

support CartesianIndices as axes for TileIterator #26

johnnychen94 opened this issue Nov 6, 2020 · 2 comments · May be fixed by #27

Comments

@johnnychen94
Copy link
Member

Currently, the axes argument only accepts tuple of ranges, and eltype of TileIteration is a tuple of ranges.

A = rand(200, 50);

R = TileIterator(axes(A), (128,8))
A[R[1]...]

It can be convenient to accept CartesianIndices here and make the eltype be CartesianIndices so that following usage is possible:

R = TileIterator(CartesianIndices(A), (128,8))
A[R[1]]
@timholy
Copy link
Member

timholy commented Nov 9, 2020

I haven't thought through all the implications but this seems quite logical. We might not have had CartesianIndices when this package was created, not sure.

@johnnychen94
Copy link
Member Author

It's a bit ugly to display this if using #27:

julia> R = TiledIndices(CartesianIndices((15, 10)), (5, 5))
3×2 TiledIndices{2, Int64, CartesianIndices{1, Tuple{Base.OneTo{Int64}}}}:
 CartesianIndex{2}[CartesianIndex(1, 1) CartesianIndex(1, 2)  CartesianIndex(1, 5) CartesianIndex(1, 6); CartesianIndex(2, 1) CartesianIndex(2, 2)  CartesianIndex(2, 5) CartesianIndex(2, 6);  ; CartesianIndex(5, 1) CartesianIndex(5, 2)  CartesianIndex(5, 5) CartesianIndex(5, 6); CartesianIndex(6, 1) CartesianIndex(6, 2)  CartesianIndex(6, 5) CartesianIndex(6, 6)]                    CartesianIndex{2}[CartesianIndex(1, 6) CartesianIndex(1, 7)  CartesianIndex(1, 9) CartesianIndex(1, 10); CartesianIndex(2, 6) CartesianIndex(2, 7)  CartesianIndex(2, 9) CartesianIndex(2, 10);  ; CartesianIndex(5, 6) CartesianIndex(5, 7)  CartesianIndex(5, 9) CartesianIndex(5, 10); CartesianIndex(6, 6) CartesianIndex(6, 7)  CartesianIndex(6, 9) CartesianIndex(6, 10)]
 CartesianIndex{2}[CartesianIndex(6, 1) CartesianIndex(6, 2)  CartesianIndex(6, 5) CartesianIndex(6, 6); CartesianIndex(7, 1) CartesianIndex(7, 2)  CartesianIndex(7, 5) CartesianIndex(7, 6);  ; CartesianIndex(10, 1) CartesianIndex(10, 2)  CartesianIndex(10, 5) CartesianIndex(10, 6); CartesianIndex(11, 1) CartesianIndex(11, 2)  CartesianIndex(11, 5) CartesianIndex(11, 6)]             CartesianIndex{2}[CartesianIndex(6, 6) CartesianIndex(6, 7)  CartesianIndex(6, 9) CartesianIndex(6, 10); CartesianIndex(7, 6) CartesianIndex(7, 7)  CartesianIndex(7, 9) CartesianIndex(7, 10);  ; CartesianIndex(10, 6) CartesianIndex(10, 7)  CartesianIndex(10, 9) CartesianIndex(10, 10); CartesianIndex(11, 6) CartesianIndex(11, 7)  CartesianIndex(11, 9) CartesianIndex(11, 10)]
 CartesianIndex{2}[CartesianIndex(11, 1) CartesianIndex(11, 2)  CartesianIndex(11, 5) CartesianIndex(11, 6); CartesianIndex(12, 1) CartesianIndex(12, 2)  CartesianIndex(12, 5) CartesianIndex(12, 6);  ; CartesianIndex(14, 1) CartesianIndex(14, 2)  CartesianIndex(14, 5) CartesianIndex(14, 6); CartesianIndex(15, 1) CartesianIndex(15, 2)  CartesianIndex(15, 5) CartesianIndex(15, 6)]     CartesianIndex{2}[CartesianIndex(11, 6) CartesianIndex(11, 7)  CartesianIndex(11, 9) CartesianIndex(11, 10); CartesianIndex(12, 6) CartesianIndex(12, 7)  CartesianIndex(12, 9) CartesianIndex(12, 10);  ; CartesianIndex(14, 6) CartesianIndex(14, 7)  CartesianIndex(14, 9) CartesianIndex(14, 10); CartesianIndex(15, 6) CartesianIndex(15, 7)  CartesianIndex(15, 9) CartesianIndex(15, 10)]

Perhaps a good reason to simplify how CartesianIndices is displayed? CartesianIndex(1, 1):CartesianIndex(6, 6) maybe?

@johnnychen94 johnnychen94 linked a pull request Dec 6, 2020 that will close this issue
3 tasks
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.

2 participants