Skip to content

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

Merged
rfourquet merged 1 commit intomasterfrom
rf/multiappend
Oct 11, 2020
Merged

allow passing multiple arguments to append! & prepend!#36227
rfourquet merged 1 commit intomasterfrom
rf/multiappend

Conversation

@rfourquet
Copy link
Copy Markdown
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 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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member

Keno commented Jun 18, 2020

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

@StefanKarpinski
Copy link
Copy Markdown
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
Copy Markdown
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 <40514306+aviatesk@users.noreply.github.com>
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 <40514306+aviatesk@users.noreply.github.com>
(cherry picked from commit 9477472)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

collections Data structures holding multiple items, e.g. sets

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants