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

Add append(_, _) and ++ #56943

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

Add append(_, _) and ++ #56943

wants to merge 1 commit into from

Conversation

jariji
Copy link
Contributor

@jariji jariji commented Jan 4, 2025

Draft for #53040

Adds append(_, _) and ++ to concatenate two sequences.

@nsajko nsajko added feature Indicates new feature / enhancement requests collections Data structures holding multiple items, e.g. sets labels Jan 4, 2025
@LilithHafner LilithHafner added the triage This should be discussed on a triage call label Jan 4, 2025
@LilithHafner
Copy link
Member

Tagging triage to discuss a substantial new feature

@adienes
Copy link
Contributor

adienes commented Jan 4, 2025

how does this differ from Iterators.flatten ?

@jariji
Copy link
Contributor Author

jariji commented Jan 4, 2025

how does this differ from Iterators.flatten ?

Iterators.flatten(::Iterator{Iterator})::Iterator always returns an Iterator. append(::T, ::T) -> T returns something like its input type.

@aplavin
Copy link
Contributor

aplavin commented Jan 4, 2025

It would be nice indeed to have a function that just concatenates two collections! Such a simple building block was surprisingly missing from Julia afaik.

@adienes
Copy link
Contributor

adienes commented Jan 4, 2025

Such a simple building block was surprisingly missing from Julia

well there are cat, stack, union, join, collect ∘ flatten etc..

I am not sure that I am convinced there needs to exist a singular overarching stick-em-together function if the value-add is primarily "magic"ness vs using a more bespoke function for the object.

and to that end I still don't really get what this would actually mean semantically. for example append! demands that the collections be ordered, but the linked issue describes append(::Set, ::Set) as being synonymous to union

and similarly states

for Matrix arguments it's hcat

which I don't really get because I thought this was supposed to be like a type-stable flatten, and that wouldn't care about dimensions?

anyway I do not actually feel too strongly about the addition of a function like this or not; I would just hope it's added because it's filling a real need rather than just tweaking some of the conventions chosen for the existing functions (e.g. the concern about cat accepting scalars, while I agree that's unfortunate, is pretty much just cosmetic IMO)

I would definitely definitely say that ++ should not be used for this though.

@aplavin
Copy link
Contributor

aplavin commented Jan 4, 2025

well there are cat, stack, union, join, collect ∘ flatten etc..

Neither of them does "concatenate two collections" I think. Even when we limit ourselves to just two common Base types, tuples and vectors.

@adienes
Copy link
Contributor

adienes commented Jan 4, 2025

tuples and vectors (any 1d array type, wouldn't have to be necessarily Vector) definitely make sense to me as the primary use case. I'm just trying to understand what the function means in a generic sense.

the PR as written seems to be saying append(x::T, y::T) where T = T(collect(Iterators.flatten((x, y)))) which makes total sense to me. I'm getting stuck on understanding what's in store for multidimensional arrays or promotion rules, but maybe that's no longer in the scope of the PR

@mcabbott
Copy link
Contributor

mcabbott commented Jan 4, 2025

The big question seems to be: In what context would you have code which wants to be generic over things this function accepts, rather than just the things accepted by vcat or join or merge.

There is some code generic over Union{AbstractVector, Tuple} in base, is that it?

What's downstream of this has to do something beyond iterating, else Iterators.flatten((a, b)) seems fine... what is it doing, in these contexts, that wants String and Vector and Tuple?

Then are lots of detail questions, like is append(x::T, y::T) where T intended to be literally the code, or is this just a sketch? That's a place it differs from vcat... e.g. what do append(1:3, 4:5) and append(1:3, 5:6) and append(1:3, 4:5.0) return, which are errors?

:d
```
"""
(append(x::T, y::T) where T<: AbstractVector) = append!(copy(x), y)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should probably be copymutable?

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 feature Indicates new feature / enhancement requests triage This should be discussed on a triage call
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants