Skip to content

Conversation

@matthias314
Copy link
Contributor

This PR makes three changes to the way any and all calls are transformed into 3-argument _any and _all calls for AbstractArray:

  • any(a) is transformed to any(identity, a) instead of calling 2-argument _any, and analogously for all. I think this has the following advantage: If somebody wants to implement any for a custom AbstractArray type, then it is now enough to define the 2-argument any. Currently also the 1-argument version has to be defined explicitly.

  • The restriction that the predicate f for the 2-argument versions of any and all be of type Function has been removed. None of the other methods for the 2-argument versions of any and all impose this, and I see no reason to have it here.

  • I've deleted the explicit definitions of the 2-argument versions of _any and _all. They are covered by the following line

    $(_fname)(A, dims; kw...) = $(_fname)(identity, A, dims; kw...)

    in the for loop a few lines later. (I believe that it doesn't matter anyway. As far as I can tell, the 2-argument _any and _all were only used for the 1-argument any and all and nowhere else.)

@mbauman
Copy link
Member

mbauman commented Sep 3, 2024

This change looks like a great idea!

I don't fully understand (yet) why any and all are defined differently than the above loop... if there is a reason to it at all. It seems like they could — and should — all be defined in exactly the same manner. And then it'd be nice to effect this change for all the reduction functions by re-jiggering the loop above in the same way you did here.

@mbauman mbauman added the fold sum, maximum, reduce, foldl, etc. label Sep 3, 2024
@matthias314
Copy link
Contributor Author

Do you want to move forward with this? If yes, do you want changes for other reduction function to be part of the same PR or separate? (I would prefer the second option.)

@jishnub
Copy link
Member

jishnub commented Sep 17, 2024

It's probably better to do these separately

@nsajko nsajko added the arrays [a, r, r, a, y, s] label Oct 4, 2024
@DilumAluthge
Copy link
Member

I see the forget me not label on this PR. Is this PR still needed? If so, @matthias314 could you rebase and fix the merge conflicts?

@matthias314
Copy link
Contributor Author

I would say it's still needed. I've just rebased it.

@adienes
Copy link
Member

adienes commented Oct 3, 2025

just for posterity & to preserve the discussion, I assume this PR is motivated by #9498 (comment) ?

@matthias314
Copy link
Contributor Author

I assume this PR is motivated by #9498 (comment) ?

No, it is motivated by my experience with developing SmallCollections.jl. The BitArray issue is something that I realized along the way and that it not affected by this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arrays [a, r, r, a, y, s] fold sum, maximum, reduce, foldl, etc. status: waiting for PR reviewer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants