Skip to content

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
60 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
@mbauman mbauman added the bugfix This change fixes an existing bug label Jun 16, 2025
KristofferC pushed a commit that referenced this pull request Jul 8, 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
julia> r = StepRangeLen(CartesianIndex(1), CartesianIndex(1), 3);

julia> r[1] in r
true

julia> CartesianIndex(0) in r
false
```

(cherry picked from commit 24b3273)
KristofferC pushed a commit that referenced this pull request Jul 8, 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
julia> r = StepRangeLen(CartesianIndex(1), CartesianIndex(1), 3);

julia> r[1] in r
true

julia> CartesianIndex(0) in r
false
```

(cherry picked from commit 24b3273)
@KristofferC
Copy link
Member

Test doesn't pass backported

cartesian                                        (9) |         failed at 2025-07-08T03:12:22.065
Test Failed at /Users/julia/.julia/scratchspaces/a66863c6-20e8-4ff4-8a62-49f30b1f605e/agent-cache/default-honeycrisp-HL2F7YQ3XH.0/build/default-honeycrisp-HL2F7YQ3XH-0/julialang/julia-release-1-dot-12/julia-c4e298cb97/share/julia/test/cartesian.jl:581
  Expression: sprint(show, c) == "CartesianIndex(3)"
   Evaluated: "CartesianIndex(3,)" == "CartesianIndex(3)"
Error During Test at /Users/julia/.julia/scratchspaces/a66863c6-20e8-4ff4-8a62-49f30b1f605e/agent-cache/default-honeycrisp-HL2F7YQ3XH.0/build/default-honeycrisp-HL2F7YQ3XH-0/julialang/julia-release-1-dot-12/julia-c4e298cb97/share/julia/test/cartesian.jl:588
  Test threw exception
  Expression: I[begin] == I[1]
  MethodError: no method matching firstindex(::CartesianIndex{2})
  The function `firstindex` exists, but no method is defined for this combination of argument types.
  Closest candidates are:
    firstindex(::Any, !Matched::Any)
     @ Base abstractarray.jl:450
    firstindex(!Matched::Base.JuliaSyntax.SourceFile)
     @ Base /Users/julia/.julia/scratchspaces/a66863c6-20e8-4ff4-8a62-49f30b1f605e/agent-cache/default-honeycrisp-HL2F7YQ3XH.0/build/default-honeycrisp-HL2F7YQ3XH-0/julialang/julia-release-1-dot-12/base/JuliaSyntax/src/source_files.jl:260
    firstindex(!Matched::Cmd)
     @ Base process.jl:716
    ...
  Stacktrace:
   [1] top-level scope
     @ /Users/julia/.julia/scratchspaces/a66863c6-20e8-4ff4-8a62-49f30b1f605e/agent-cache/default-honeycrisp-HL2F7YQ3XH.0/build/default-honeycrisp-HL2F7YQ3XH-0/julialang/julia-release-1-dot-12/julia-c4e298cb97/share/julia/test/cartesian.jl:587
   [2] macro expansion
     @ /Users/julia/.julia/scratchspaces/a66863c6-20e8-4ff4-8a62-49f30b1f605e/agent-cache/default-honeycrisp-HL2F7YQ3XH.0/build/default-honeycrisp-HL2F7YQ3XH-0/julialang/julia-release-1-dot-12/julia-c4e298cb97/share/julia/stdlib/v1.12/Test/src/Test.jl:1776 [inlined]
   [3] macro expansion
     @ /Users/julia/.julia/scratchspaces/a66863c6-20e8-4ff4-8a62-49f30b1f605e/agent-cache/default-honeycrisp-HL2F7YQ3XH.0/build/default-honeycrisp-HL2F7YQ3XH-0/julialang/julia-release-1-dot-12/julia-c4e298cb97/share/julia/test/cartesian.jl:588 [inlined]
   [4] macro expansion
     @ /Users/julia/.julia/scratchspaces/a66863c6-20e8-4ff4-8a62-49f30b1f605e/agent-cache/default-honeycrisp-HL2F7YQ3XH.0/build/default-honeycrisp-HL2F7YQ3XH-0/julialang/julia-release-1-dot-12/julia-c4e298cb97/share/julia/stdlib/v1.12/Test/src/Test.jl:677 [inlined]
Error During Test at /Users/julia/.julia/scratchspaces/a66863c6-20e8-4ff4-8a62-49f30b1f605e/agent-cache/default-honeycrisp-HL2F7YQ3XH.0/build/default-honeycrisp-HL2F7YQ3XH-0/julialang/julia-release-1-dot-12/julia-c4e298cb97/share/julia/test/cartesian.jl:589
  Test threw exception
  Expression: I[end] == I[2]
  MethodError: no method matching lastindex(::CartesianIndex{2})
  The function `lastindex` exists, but no method is defined for this combination of argument types.
  Closest candidates are:
    lastindex(::Any, !Matched::Any)
     @ Base abstractarray.jl:427
    lastindex(!Matched::Base.JuliaSyntax.SourceFile)
     @ Base /Users/julia/.julia/scratchspaces/a66863c6-20e8-4ff4-8a62-49f30b1f605e/agent-cache/default-honeycrisp-HL2F7YQ3XH.0/build/default-honeycrisp-HL2F7YQ3XH-0/julialang/julia-release-1-dot-12/base/JuliaSyntax/src/source_files.jl:261
    lastindex(!Matched::Cmd)
     @ Base process.jl:716
    ...
  Stacktrace:
   [1] top-level scope
     @ /Users/julia/.julia/scratchspaces/a66863c6-20e8-4ff4-8a62-49f30b1f605e/agent-cache/default-honeycrisp-HL2F7YQ3XH.0/build/default-honeycrisp-HL2F7YQ3XH-0/julialang/julia-release-1-dot-12/julia-c4e298cb97/share/julia/test/cartesian.jl:587
   [2] macro expansion
     @ /Users/julia/.julia/scratchspaces/a66863c6-20e8-4ff4-8a62-49f30b1f605e/agent-cache/default-honeycrisp-HL2F7YQ3XH.0/build/default-honeycrisp-HL2F7YQ3XH-0/julialang/julia-release-1-dot-12/julia-c4e298cb97/share/julia/stdlib/v1.12/Test/src/Test.jl:1776 [inlined]
   [3] macro expansion
     @ /Users/julia/.julia/scratchspaces/a66863c6-20e8-4ff4-8a62-49f30b1f605e/agent-cache/default-honeycrisp-HL2F7YQ3XH.0/build/default-honeycrisp-HL2F7YQ3XH-0/julialang/julia-release-1-dot-12/julia-c4e298cb97/share/julia/test/cartesian.jl:589 [inlined]
   [4] macro expansion
     @ /Users/julia/.julia/scratchspaces/a66863c6-20e8-4ff4-8a62-49f30b1f605e/agent-cache/default-honeycrisp-HL2F7YQ3XH.0/build/default-honeycrisp-HL2F7YQ3XH-0/julialang/julia-release-1-dot-12/julia-c4e298cb97/share/julia/stdlib/v1.12/Test/src/Test.jl:677 [inlined]

KristofferC added a commit that referenced this pull request Jul 8, 2025
@KristofferC KristofferC removed the backport 1.12 Change should be backported to release-1.12 label Jul 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This change fixes an existing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants