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

eachindex should throw DimensionMismatch for differently sized tuples #47898

Open
cafaxo opened this issue Dec 14, 2022 · 7 comments
Open

eachindex should throw DimensionMismatch for differently sized tuples #47898

cafaxo opened this issue Dec 14, 2022 · 7 comments
Labels
bug Indicates an unexpected problem or unintended behavior correctness bug ⚠ Bugs that are likely to lead to incorrect results in user code without throwing good first issue Indicates a good issue for first-time contributors to Julia

Comments

@cafaxo
Copy link
Contributor

cafaxo commented Dec 14, 2022

Currently, we have

julia> eachindex((), (1,))
Base.OneTo(1)

From the documentation of eachindex, I expected a DimensionMismatch instead. Like this:

julia> eachindex(1:0, 1:1)
ERROR: DimensionMismatch: all inputs to eachindex must have the same indices, got Base.OneTo(0) and Base.OneTo(1)

(Previous discussion on slack: https://julialang.slack.com/archives/C6A044SQH/p1671032794309049)

@fredrikekre
Copy link
Member

Current behavior introduced in #14100, when eachindex returned the max, and #25108 missed to update this to the new behavior for tuples.

@fredrikekre fredrikekre added bug Indicates an unexpected problem or unintended behavior good first issue Indicates a good issue for first-time contributors to Julia labels Dec 15, 2022
@cafaxo
Copy link
Contributor Author

cafaxo commented Dec 16, 2022

This also seems to behave incorrectly:

julia> itr0 = (i for i in 1:0);

julia> itr1 = (i for i in 1:1);

julia> eachindex(itr0)
0-element LinearIndices{1, Tuple{Base.OneTo{Int64}}}

julia> eachindex(itr1)
1-element LinearIndices{1, Tuple{Base.OneTo{Int64}}}:
 1

julia> eachindex(itr0, itr1)
ERROR: MethodError: no method matching keys(::Base.Generator{UnitRange{Int64}, typeof(identity)}, ::Base.Generator{UnitRange{Int64}, typeof(identity)})

Not sure if eachindex even makes sense for generators... but then it should give Base.OneTo instead of LinearIndices.
Should keys(::Tuple) give LinearIndices to be consistent with keys(::Generator) and keys(::AbstractArray)?
(keys(::Generator) was introduced in #34678)

@Snimm
Copy link

Snimm commented Dec 24, 2022

Hi, I am new to coding, and I want to work on this issue.

@piyushjain16
Copy link

Is the issue closed ?. if not I would like to work on it.

@oscardssmith
Copy link
Member

not closed, but #48125 looks like it will fix this once it merges.

@piyushjain16
Copy link

Ok thank you

@uroosa114
Copy link

I would like to work on this

@LilithHafner LilithHafner added the correctness bug ⚠ Bugs that are likely to lead to incorrect results in user code without throwing label Aug 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior correctness bug ⚠ Bugs that are likely to lead to incorrect results in user code without throwing good first issue Indicates a good issue for first-time contributors to Julia
Projects
None yet
Development

No branches or pull requests

7 participants