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

implement mapreduce(f, op) without any collection input for disambiguity #53360

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

putianyi889
Copy link
Contributor

Although unintended (I guess), currently mapreduce(f, op) can work, returning f(). However this can easily cause ambiguity if mapreduce tries to support custom types.

For example,

julia> mapreduce(eps, +)
2.220446049250313e-16

julia> using QuasiArrays

julia> mapreduce(eps,+)
ERROR: MethodError: mapreduce(::typeof(eps), ::typeof(+)) is ambiguous.

Candidates:
  mapreduce(f, op, A::AbstractQuasiArray...; kw...)
    @ QuasiArrays C:\Users\pty\.julia\dev\QuasiArrays\src\quasireducedim.jl:142
  mapreduce(f, op, A::Union{Base.AbstractBroadcasted, AbstractArray}...; kw...)
    @ Base reducedim.jl:359

Possible fix, define
  mapreduce(::Any, ::Any)

Stacktrace:
 [1] top-level scope
   @ REPL[12]:1

@mikmoore
Copy link
Contributor

mikmoore commented Feb 16, 2024

The issue is that QuasiArrays is committing piracy. Vararg inputs must always consider the possibility of 0 arguments and the piracy potential it creates. I think the more durable solution is to make a PR to QuasiArrays, removing the piracy by redefining the offending signature as

function mapreduce(f, op, A::AbstractQuasiArray, B::AbstractQuasiArray...; kw...)

Also, #52631 recently removed map's 0-collection (i.e., map(f)) signature. I expect that mapreduce should follow this same lead, although perhaps there is some reason it's different and shouldn't?

@putianyi889
Copy link
Contributor Author

Also, #52631 recently removed map's 0-collection (i.e., map(f)) signature.

That's interesting to know and it will make f(A::something, B::something...) a standard way to do things. However, I still prefer to have a f() in Base. All it does is to define f as accepting at least one argument, allowing everyone else to use the shorter f(A::something...) without a problem.

@nsajko nsajko added invalid Indicates that an issue or pull request is no longer relevant fold sum, maximum, reduce, foldl, etc. and removed invalid Indicates that an issue or pull request is no longer relevant labels Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fold sum, maximum, reduce, foldl, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants