Skip to content

Define in for CartesianIndex ranges #58616

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

Merged
merged 1 commit into from
Jun 16, 2025

Conversation

jishnub
Copy link
Member

@jishnub jishnub commented Jun 3, 2025

Currently, this hits a fallback method that assumes that division is defined for the elements of the range. After this, the following works:

julia> r = StepRangeLen(CartesianIndex(1), CartesianIndex(1), 3);

julia> r[1] in r
true

julia> CartesianIndex(0) in r
false

@jishnub jishnub force-pushed the jishnub/cartesianindex_in_steprangelen branch from c14852c to 38c0ef1 Compare June 3, 2025 12:10
@jishnub jishnub added the backport 1.12 Change should be backported to release-1.12 label Jun 3, 2025
@Seelengrab
Copy link
Contributor

Ref #57034

This PR seems to reinforce the view that ranges are collections of objects, and not intervals. I'm not sure it's relevant, but I think I remember people being of two minds about this, so I thought I'd mention it.

@jishnub
Copy link
Member Author

jishnub commented Jun 4, 2025

For what it's worth, this PR was motivated by failing to compute the intersection of these ranges, which is used in checking for aliasing. I believe intersect treats ranges as collections, as it iterates over them.

Closes #57034, and the suggested implementation in that issue is pretty similar to this one.

@KristofferC KristofferC mentioned this pull request Jun 6, 2025
55 tasks
@jishnub jishnub merged commit 24b3273 into master Jun 16, 2025
8 checks passed
@jishnub jishnub deleted the jishnub/cartesianindex_in_steprangelen branch June 16, 2025 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 1.12 Change should be backported to release-1.12 bugfix This change fixes an existing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants