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

Consider making push/pushfirst/insert widen the type #732

Open
c42f opened this issue Feb 17, 2020 · 5 comments
Open

Consider making push/pushfirst/insert widen the type #732

c42f opened this issue Feb 17, 2020 · 5 comments
Labels
arrays AbstractArray interface and array operations

Comments

@c42f
Copy link
Member

c42f commented Feb 17, 2020

In #702 (comment) @tkf suggested that it would be helpful to have push promote the type. I'm opening this issue because while it's easy to implement I don't have a strong feeling for the benefits and drawbacks.

The main drawback I see is that it's quite different from push! and might be surprising if b = push(a, element) gives b with a different eltype from a.

But other than quibble it seems natural for functional manipulation of immutable data structures if the type is promoted. (Wrinkle: for consistency with StaticArrays constructors this would have to be promotion via promote_type rather than pure widening, otherwise we'll quickly get abstract types in numeric code.)

If we did this, we should also make pushfirst and insert the same for consistency.

@tkf
Copy link
Member

tkf commented Feb 17, 2020

While we are at it... I think it's worth considering to use widening semantics even for setindex. This might be much more controversial, but I think there are non-contrived code snippets with element type progression Bool -> Float64 -> ComplexF64 or Float64 -> Dual.

@c42f
Copy link
Member Author

c42f commented Feb 17, 2020

If we do push I think we could also do setindex, I feel like it's the same kind of weird-but-maybe-sensible :-)

If you've got a few concrete use cases it would be great to see them.

@tkf
Copy link
Member

tkf commented Feb 17, 2020

Partition, DropLast, and TakeLast in Transducers.jl use setindex!! (setindex/setindex! with widening) to manipulate the buffer vector. So, in principle, I can use SVector as the buffer. But I'm not sure if it's worth doing this as a tuple could be an OK solution for the length that SVector works? Other than this, I can't find existing use-cases for type-widening setindex on vectors.

@c42f
Copy link
Member Author

c42f commented Feb 18, 2020

Right thanks! I was more wondering about concrete use cases for push? Trying to get a feel for how compelling those are vs the alternative.

@tkf
Copy link
Member

tkf commented Feb 18, 2020

Type-widening push (called via maybe-mutating version push!!/append!!) is the main work-horse of the collect-like functions in Transducers.jl. A self-contained example would be something like this:

using BangBang

# Something like `Base.map` but you can specify the output array type:
mapto(f, T, xs) = mapfoldl(f, push!!, xs; init=T{Union{}}())

mapto(inv, Vector, 1:4)
mapto(inv, SVector{0}, 1:4)

# eltype changes Union{} -> Int -> Union{Missing,Int}
mapto(x -> isodd(x) ? missing : x, SVector{0}, 1:4)

This is just a trivial example but I think this would be useful when doing something much more complex; e.g., defining static tables.

FYI, there is a request for standard API for push_widen! pattern in Base JuliaLang/julia#34478. I think it's a related discussion if static arrays should support this (even though all of them currently have immutable shape).

Other related discussions:

@c42f c42f added the arrays AbstractArray interface and array operations label Apr 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrays AbstractArray interface and array operations
Projects
None yet
Development

No branches or pull requests

2 participants