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

allow passing multiple arguments to append! & prepend! #36227

Merged
merged 1 commit into from
Oct 11, 2020

Conversation

rfourquet
Copy link
Member

@rfourquet rfourquet commented Jun 11, 2020

E.g. append!([1], [2], [3]) == [1, 2, 3].
The equivalent operations for sets (union!) and dictionaries (merge!)
already support multiple arguments, as well as push!.

For prepend!, order is maintained in the same way as in pushfirst!:
prepend!([3], [1], [2]) == [1, 2, 3] == pushfirst!([3], 1, 2).

@rfourquet rfourquet added domain:collections Data structures holding multiple items, e.g. sets needs news A NEWS entry is required for this change labels Jun 11, 2020
Copy link
Member

@tkf tkf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Keno
Copy link
Member

Keno commented Jun 18, 2020

What's the rational for the prepend! behavior? Usually when the function modify their first argument, we have varargs behave as lfold.

@tkf
Copy link
Member

tkf commented Jun 18, 2020

append!(xs, a, b, c) is equivalent to push!(xs, a..., b..., c...). So, it makes sense if prepend!(xs, a, b, c) is equivalent to pushfirst!(xs, a..., b..., c...), as discussed in the OP.

It might make sense if pushfirst! had left-fold behavior but that's too late now.

@Keno
Copy link
Member

Keno commented Jun 18, 2020

Alright I buy that, but @StefanKarpinski or @JeffBezanson want to give a quick second opinion?

@StefanKarpinski
Copy link
Sponsor Member

Agree with what @tkf said, including the rationale and the potential regret.

@rfourquet rfourquet removed the needs news A NEWS entry is required for this change label Oct 4, 2020
@rfourquet
Copy link
Member Author

News just added. As I think it's an uncontroversial change, I will merge soon.

@rfourquet rfourquet closed this Oct 10, 2020
@rfourquet rfourquet reopened this Oct 10, 2020
E.g. append!([1], [2], [3]) == [1, 2, 3].
The equivalent operation for sets (`union!`) and dictionaries (`merge!`)
already supports multiple arguments, as well as `push!`.

For `prepend!`, order is maintained in the same way as in `pushfirst!`:
`prepend!([3], [1], [2]) == [1, 2, 3] == pushfirst!([3], 1, 2)`.
@rfourquet rfourquet merged commit 0c5329c into master Oct 11, 2020
@rfourquet rfourquet deleted the rf/multiappend branch October 11, 2020 12:44
vtjnash added a commit that referenced this pull request Jun 6, 2024
Attempt to fix #54711
Test introduced by #36227
vtjnash added a commit that referenced this pull request Jun 6, 2024
Attempt to fix #54711
Test introduced by #36227
vtjnash added a commit that referenced this pull request Jun 6, 2024
Attempt to fix #54711
Test introduced by #36227
aviatesk added a commit that referenced this pull request Jun 7, 2024
Attempt to fix #54711
Test introduced by #36227

---------

Co-authored-by: Shuhei Kadowaki <[email protected]>
KristofferC pushed a commit that referenced this pull request Jun 13, 2024
Attempt to fix #54711
Test introduced by #36227

---------

Co-authored-by: Shuhei Kadowaki <[email protected]>
(cherry picked from commit 9477472)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:collections Data structures holding multiple items, e.g. sets
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants