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

findmax does different things on Julia 1.6 and 1.7 with the same version of Compat #745

Closed
ericphanson opened this issue Jun 23, 2021 · 5 comments · Fixed by #748
Closed

Comments

@ericphanson
Copy link

ericphanson commented Jun 23, 2021

❯ julia-1.7 -q
julia> using Compat # v3.31.0

julia> findmax(identity, 5:9)
(9, 5)

julia> VERSION
v"1.7.0-beta2"

❯ julia-1.6 -q
julia> using Compat # v3.31.0

julia> findmax(identity, 5:9)
(9, 9)

julia> VERSION
v"1.6.1"

This is, of course, since the choice of what findmax does changed between when #738 was made and the 1.7-beta2 release, and in Compat, the definitions are gated around versions, so on 1.7 the real definition is let through, while on 1.6, the Compat definition is used.

But how do we fix this?

@fredrikekre
Copy link
Member

Just update to match 1.7-beta?

@ericphanson
Copy link
Author

Yeah, so Compat v4 can do that, and I guess v3 is just going to be inconsistent?

@fredrikekre
Copy link
Member

Why not in v3? This is a bug in Compat.

@ericphanson
Copy link
Author

ericphanson commented Jun 23, 2021

Is it though? Changing the definition basically means any code using Compat for the findmax methods added in #738 will break (likely silently), and it was very explicit about what those methods did.

Compat tagged a release with those methods, Julia did not, so Julia can change them, but I don't think Compat can...

@fredrikekre
Copy link
Member

I guess it was premature to tag the Compat release, but the job of Compat is to behave like Julia, and there have not been a Julia release with this behavior yet, so I would 100% call this a bug in Compat.

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