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

argmin/argmax not defined for Tuples and maybe others #621

Closed
anowacki opened this issue Aug 22, 2018 · 2 comments
Closed

argmin/argmax not defined for Tuples and maybe others #621

anowacki opened this issue Aug 22, 2018 · 2 comments

Comments

@anowacki
Copy link
Contributor

Although argmin/argmax on v0.7+ are defined on anything you can iterate (via findmin/findmax), that's not the case within Compat at the moment. Currently, they are defined for:

  1. AbstractArray
  2. AbstractVector
  3. Associative

Therefore, you can't currently write compatible code if you want to use Tuples:

julia> VERSION
v"0.6.3"

julia> using Compat

julia> argmin((1, 2, 3.0))
ERROR: MethodError: no method matching argmin(::Tuple{Int64,Int64,Int64})
Closest candidates are:
  argmin(::AbstractArray{T,1} where T) at /Users/nowacki/.julia/v0.6/Compat/src/Compat.jl:1401
  argmin(::AbstractArray) at /Users/nowacki/.julia/v0.6/Compat/src/Compat.jl:1400
  argmin(::Associative) at /Users/nowacki/.julia/v0.6/Compat/src/Compat.jl:1402

Works fine with indmin/indmax, of course:

julia> indmin((1, 2, 3.0))
1
anowacki added a commit to anowacki/Compat.jl that referenced this issue Aug 22, 2018
Previously, only a limited set of input collections were supported
with argmin/argmax.  Add a fallback to indmin/indmax for remaining
types, which allows them to work on Tuples.

Fixes JuliaLang#621.
@anowacki
Copy link
Contributor Author

Potential remedy at #622. It's possible there may be types for which this should not be defined, but for argmin/argmax in Base in v0.7+ Any is accepted, so I think it should be okay.

anowacki added a commit to anowacki/Compat.jl that referenced this issue Aug 22, 2018
martinholters pushed a commit that referenced this issue Aug 23, 2018
@martinholters
Copy link
Member

This is only fixed for the Tuple part, not for the "and maybe others", because there is not necessarily a generally valid way to implement this in 0.6. But we can re-open this issue or open another one if the need for a specific "other" one arises.

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

No branches or pull requests

2 participants