return on push (and other dequeue functions)... why not push! instead ?

[Diego Javier Zea]

push modifies the input Array and returns the modified Array too.
Given the modification on input, why is not push!
return is not redundant?

julia> ie = [1; 2]
2-element Int64 Array:
 1
 2

julia> push(ie,3)
3-element Int64 Array:
 1
 2
 3

julia> ie
3-element Int64 Array:
 1
 2
 3

[Diego Javier Zea]

But... append! is with ! and not append.

Maybe can be dequeue functions with a return (and without modify the input) working on immutable too (strings) without ! and other without return with ! in the name for indicate modification of the input Array ?

[John Myles White]

I think push should be push!. And del should be del!.

 -- John

--
 
 
 

[Jeffrey Sarnoff]

+1

[Diego Javier Zea]

Is return the input Array of this functions necessary ? I look for situation on the Julia code where this ( redundant ? ) returns where used. I found that only in grow and del is used (and except for sparse.jl, the other used of the return value looks redundant -assigned to the new Array to itself- )

dzea@deepthought:~$ grep "=\s*push(" ./bin/julia/base/*.jl

dzea@deepthought:~$ grep "=\s*append\!(" ./bin/julia/base/*.jl
./bin/julia/base/bitarray.jl:append!(B::BitVector, items::AbstractVector{Bool}) = append!(B, bitpack(items))
./bin/julia/base/bitarray.jl:append!(A::Vector{Bool}, items::BitVector) = append!(A, bitunpack(items))

dzea@deepthought:~$ grep "=\s*grow(" ./bin/julia/base/*.jl
./bin/julia/base/linalg_sparse.jl:            rowvalC = grow(rowvalC, max(nnzC,mA))
./bin/julia/base/linalg_sparse.jl:            nzvalC = grow(nzvalC, max(nnzC,mA))

dzea@deepthought:~$ grep "=\s*enqueue(" ./bin/julia/base/*.jl
./bin/julia/base/multi.jl:enq_work(wi::WorkItem) = enqueue(Workqueue, wi)

dzea@deepthought:~$ grep "=\s*unshift(" ./bin/julia/base/*.jl

dzea@deepthought:~$ grep "=\s*insert(" ./bin/julia/base/*.jl

dzea@deepthought:~$ grep "=\s*del(" ./bin/julia/base/*.jl
./bin/julia/base/dict.jl:del{K}(wkh::WeakKeyDict{K}, key) = del(wkh.ht, key)
./bin/julia/base/linalg_sparse.jl:    rowvalC = del(rowvalC, colptrC[end]:length(rowvalC))
./bin/julia/base/linalg_sparse.jl:    nzvalC = del(nzvalC, colptrC[end]:length(nzvalC))
./bin/julia/base/multi.jl:del_fd_handler(fd::Int32) = del(fd_handlers, fd)
./bin/julia/base/sparse.jl:            rowvalS = del(rowvalS, colptrS[end]:length(rowvalS))
./bin/julia/base/sparse.jl:            nzvalCS = del(nzvalS, colptrS[end]:length(nzvalS))
./bin/julia/base/sparse.jl:    A.rowval = del(rowvalS, colptrS[end]:length(rowvalS))
./bin/julia/base/sparse.jl:    A.nzval  = del(nzvalS, colptrS[end]:length(nzvalS))

dzea@deepthought:~$ grep "=\s*del_all(" ./bin/julia/base/*.jl

dzea@deepthought:~$

[Brian Mattern]

The return allows chaining functions together (e.g., multiple push operations). So, it isn't completely redundant.

Brian

On Jan 5, 2013, at 3:30 PM, Diego Javier Zea <dieg...@gmail.com> wrote:

push modifies the input Array and returns the modified Array too.

...
return is not redundant?

[Diego Javier Zea]

Thanks Brian, I don't think in chained one inside the other!
...but you can chaining only bay execute one before the other. Don't looks that something that it's going to generate troubles...

julia> a = [ 1; 2; 3; 4; 5; 6 ]
6-element Int64 Array:
 1
 2
 3
 4
 5
 6

julia> push(push(a,7),8)
8-element Int64 Array:
 1
 2
 3
 4
 5
 6
 7
 8

julia> function push!{T}(a::Array{T,1}, item)
           if is(T,None)                                    
               error("[] cannot grow. Instead, initialize the array with \"T[]\".")
           end
           # convert first so we don't grow the array if the assignment won't work
           item = convert(T, item)
           ccall(:jl_array_grow_end, Void, (Any, Uint), a, 1)
           a[end] = item
           nothing
       end

julia> function push!(a::Array{Any,1}, item::ANY)
           ccall(:jl_array_grow_end, Void, (Any, Uint), a, 1)
           arrayset(a, item, length(a))
           nothing
       end

julia> push!(push!(a,9),10)
no method push!(Nothing,Int64)

julia> a
9-element Int64 Array:
 1
 2
 3
 4
 5
 6
 7
 8
 9

julia> push!(a,10)

julia> push!(a,11)

julia> a
11-element Int64 Array:
  1
  2
  3
  4
  5
  6
  7
  8
  9
 10
 11

[Diego Javier Zea]

For example; push inside push is not used on the code of base.

dzea@deepthought:~$ grep "push.*push" ./bin/julia/base/*.jl
./bin/julia/base/git.jl:            run(`git config remote.origin.pushurl $pushurl`)

[Diego Javier Zea]

Without the return, and with nothing instead looks to be a little more faster too ;)
[ for example, the push! definition without return - on previous messages - its a little more faster than the actual push ]

The mean with return is:   4.70e-5 +- 0.045e-5
And without return:        4.56e-5 +- 0.049e-5

Mean elapsed time			N on accumulated elapsed time * 1000 < 120
With return	Without return		With return	Without return
4.75E-05	4.58E-05	1	2528866	2620081	1
4.64E-05	4.63E-05	1	2585591	2590230	1
4.68E-05	4.39E-05	1	2563664	2732904	1
4.88E-05	4.43E-05	1	2460211	2707355	1
4.73E-05	4.49E-05	1	2537059	2671809	1
4.37E-05	4.38E-05	0	2746197	2737611	0
4.39E-05	4.24E-05	1	2735401	2833182	1
4.90E-05	4.23E-05	1	2448759	2839539	1
4.87E-05	4.94E-05	0	2463836	2427746	0
4.86E-05	4.75E-05	1	2470752	2523721	1
4.87E-05	4.74E-05	1	2466148	2533975	1
4.82E-05	4.75E-05	1	2487281	2524607	1
4.51E-05	4.71E-05	0	2658315	2546760	0
4.57E-05	4.49E-05	1	2624526	2671443	1
4.54E-05	4.55E-05	0	2645352	2634962	0
4.89E-05	4.57E-05	1	2455947	2624691	1
		0.75			0.75

[Diego Javier Zea]

The code of the previous measure (running 2 times, altering the order of execution between one measure and the other):

macro timeit(ex)
    quote
        gc()
        t_cum = 0
        i=1
        t=Float64[]
        while t_cum*1000 < 120
            push(t,@elapsed $ex)
            t_cum += t[i]
            i += 1
        end
    t .*= 1000
    println("Mean\tSD\tN\tMin:\t$(Base.mean(t))\t$(Base.std(t))\t$(i-1)\t$(Base.min(t))")
    end
end
a=[1;2]
push([1;2],3)
a
a=[1;2]
push!([1;2],3)
a
for i in 1:2
 println("---------------------------------------------------")
 a=[1:100]
 print("without return:\t")
 @timeit push!(a,1)
 a=[1:100]
 print("with return:\t")
 @timeit push(a,1)  
 a=[1:100]
 print("without return:\t")
 @timeit push!(a,1)  
 a=[1:100]
 print("with return:\t")
 @timeit push(a,1)
 a=[1:100]
 print("without return:\t")
 @timeit push!(a,1)
 a=[1:100]
 print("with return:\t")
 @timeit push(a,1)  
 a=[1:100]
 print("without return:\t")
 @timeit push!(a,1)  
 print("with return:\t")
 @timeit push(a,1)
end

[Diego Javier Zea]

But... Even if the (redundant) return statement is maintained in this dequeue functions, the names must have a ! for follow the rule:

The last function, fill!, is different in that it modifies an existing array instead of constructing a new one. As a convention, functions with this property have names ending with an exclamation point. These functions are sometimes called “mutating” functions, or “in-place” functions.

[Diego Javier Zea]

I do a very simple test for push against push! in this mail list [ previous link ] yesterday at 3 AM. Excuse me... it's wrong [ it's lees than a 1% better, probably only measurement error ]. Here a test for:

function enqueue!{T}(a::Array{T,1}, item)
    if is(T,None)
        error("[] cannot grow. Instead, initialize the array with \"T[]\".")
    end
    item = convert(T, item)
    ccall(:jl_array_grow_beg, Void, (Any, Uint), a, 1)
    a[1] = item
    nothing
end

And the results are:

        BenchmarkCategory BenchmarkName Iterations Fun_wins_to_Fun2 Fun2_wins_to_Fun    Ties  TotalWall AverageWall    MaxWall MinWall             Timestamp            JuliaVersion    JuliaHash CodeHash      OS
[1,]     "compare_return"        "with"      50220          4.60024          4.78505 90.6147 0.00282693  5.62909e-8   9.799e-5     0.0 "2013-01-06 19:51:44" "0.0.0+106484954.r5387" "53872cd2c9"       NA "Linux"
[2,]     "compare_return"     "without"      49780          4.60024          4.78505 90.6147 0.00268102  5.38573e-8 5.38826e-5     0.0 "2013-01-06 19:51:44" "0.0.0+106484954.r5387" "53872cd2c9"       NA "Linux"

But, I still think that the names need to be ending in ! for indicate modification of input data.

[John Myles White]

I still support renaming push to push!.

I think allowing a return value from push is very important.

 -- John

--
 
 
 

[Stefan Karpinski]

Ok, I think there's pretty strong consensus that these mutating operations in Base should be renamed to have ! at their ends, so I'm resolving to make that change this week, giving people some warning before switching.

--
 
 
 

[Milktrader]

+1 (I thought this was already the Julian way)