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

Should append! accept any iterable? #7798

Closed
ivarne opened this issue Jul 31, 2014 · 15 comments
Closed

Should append! accept any iterable? #7798

ivarne opened this issue Jul 31, 2014 · 15 comments
Labels
status:help wanted Indicates that a maintainer wants help on an issue or pull request

Comments

@ivarne
Copy link
Sponsor Member

ivarne commented Jul 31, 2014

append! is currently limited to instances of AbstractArray. I can't really see why this restriction is necessary, as lots of similar functions gladly take any iterable in similar situations.

Somewhat related to #7642

@JeffBezanson
Copy link
Sponsor Member

For append! it helps to know the length of the iterable. We could have it fall back to append!(a, collect(itr)) or repeatedly calling push!, but those aren't particularly fast so there's not much value over writing that code yourself. But we could add these in the interest of being more generic.

@quinnj
Copy link
Member

quinnj commented Aug 4, 2014

I don't think this is particularly useful. I think in general, I would prefer append! to be strict in its usage to help me, as a user, be smart about performance.
Is there an example of an iterable where this would be useful and is non-trivial (and is not fishy on performance?)
I guess I could kind of see an application for DataFrames where you want to take a row iterator from one DataFrame and append each row to another DataFrame. In that case, you'd probably want the push! behavior.

@mcg1969
Copy link

mcg1969 commented Aug 8, 2014

For any collection that implements length, overloading append! avoids length(B) calls to :jl_array_row_end. The implementation of collect would be a good model here, which tests applicable(length,itr) to preallocate the storage:

function append!{T}(a::Array{T,1}, items::itr)
if applicable(length,itr)
    n = length(items)
    ccall(:jl_array_grow_end, Void, (Any, Uint), a, n)
    i = length(a)
    for x in itr
        a[i+=1] = x
    end
else
    for x in itr
        push!(a,x)
    end
end

prepend! wouldn't be as elegant if length is not defined:

function prepend!{T}(a::Array{T,1}, items::itr)
if applicable(length,itr)
    n = length(items)
    ccall(:jl_array_grow_beg, Void, (Any, Uint), a, n)
    i = 0
    for x in itr
        a[i+=1] = x
    end
else
    prepend!(a,collect(T,items))
end

@mcg1969
Copy link

mcg1969 commented Aug 8, 2014

This dovetails with @JeffBezanson's suggestion of a Collection abstract class. That way, only collections with the same eltype would be accepted, if such a restriction were desirable.

@mcg1969
Copy link

mcg1969 commented Aug 8, 2014

An alternative: provide a collect! parallel to collect that appends to an existing array.

@AnnaDana
Copy link

AnnaDana commented Nov 5, 2014

Is there another function that takes a variable array and add it to a higher dimension one? This can be used in implementing algorithms such as column and constraint generation.

@StefanKarpinski
Copy link
Sponsor Member

Is there another function that takes a variable array and add it to a higher dimension one?

Not quite sure I understand. Can you elaborate a bit on what you mean?

@IainNZ
Copy link
Member

IainNZ commented Nov 5, 2014

@AnnaDana nice to see another OR person using Julia! Do you you mean something like

A = rand(2,2)  # 2x2 matrix
x = rand(2)  # length 2 vector
hcat(A,x)  # 2x3 matrix

@AnnaDana
Copy link

AnnaDana commented Nov 6, 2014

Briefly, after each solve of sub problem, when I want to add a new column which is basicaly new variable, I do the following:

iteration = iteration +1
@defVar(MasterProblem, x[iteration, 1:5] >=0)

And this works fine! I was just wondering if there is a function so that I define my variable x with dynamic size once, and at each iteration add a new row to it.

@IainNZ
Copy link
Member

IainNZ commented Nov 6, 2014

@AnnaDana since this is a JuMP specific question, can you open an issue at https://github.com/JuliaOpt/JuMP.jl ?

@vtjnash
Copy link
Sponsor Member

vtjnash commented Dec 22, 2016

Seems like this should be revisited now that general iterables have a length computable attribute?

@tkelman tkelman modified the milestones: 1.0, 0.6.0 Jan 5, 2017
@JeffBezanson
Copy link
Sponsor Member

It looks like this was added by e909c1c

@Keno Keno added status:help wanted Indicates that a maintainer wants help on an issue or pull request and removed needs decision A decision on this change is needed kind:speculative Whether the change will be implemented is speculative labels Jul 20, 2017
@JeffBezanson
Copy link
Sponsor Member

We're missing append!(::BitArray, iterable). Should add that method.

@StefanKarpinski
Copy link
Sponsor Member

Adding methods is non-breaking, so this does not belong on the 1.0 milestone.

@StefanKarpinski StefanKarpinski modified the milestones: 1.x, 1.0 Aug 31, 2017
@Keno
Copy link
Member

Keno commented Apr 18, 2019

The missing method was implemented by @JeffBezanson in 4844a4b

@Keno Keno closed this as completed Apr 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:help wanted Indicates that a maintainer wants help on an issue or pull request
Projects
None yet
Development

No branches or pull requests

10 participants