consider consistently using ! for mutating functions

[StefanKarpinski]

E.g. rename push => push!, etc. Discussion here.

[diegozea]

I think the same, actually... I was thinking in open an issue like this!
I'm extending the dequeue function for my types, and the actual definitions looks awkward...
I'm think that push, del and others must be to be push!, del!, etc.
And returning the modified input in a function that modified the input looks awkward (redundant) and is inefficient (without not reason).

I'm opened this topic on julia-dev:
[https://groups.google.com/forum/?hl=es&fromgroups=#!topic/julia-dev/wKIlMaSj0YE]

[JeffBezanson]

"returning the modified input...is inefficient": Prove it.

[StefanKarpinski]

I'm not sure where the idea that returning the array is inefficient comes from, but that's just not the case.

[diegozea]

I do a very simple test for push against push! in the mail list [ previous link ] yesterday at 3 AM. But, yes... Excuse me... Looks to be my error [ 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.

[StefanKarpinski]

I'm proposing the following renames:

push    => push!
pop     => pop!
grow    => grow!
enqueue => unshift!
unshift => unshift!
insert  => insert!
del     => delete!
del_all => deleteall! (or clear!?)

As a transitional step, the old versions will be kept around with warnings.

[diegozea]

Good!! 👍

I think deleteall! is better than clear! [ clear! gives the idea of deletion of data but maintain the structure (type and size) ]

[JeffBezanson]

Here are my notes from our discussion about !, especially how it relates to linear algebra.

! convention:
after f!(A), A should equal B in B=f(A)

constructor convention:
Type(A) takes ownership of A and can mutate it.

linear algebra functions that might throw errors should be designed as follows:
a low-level function/ctor CholeskyDense(A) that takes A, mutates it, and
returns something that can tell you whether A was posdef. Does NOT throw.

a high-level function chol(A) and/or chol!(A) that throws an error if there
is a problem. it might have to do CholeskyDense(copy(A)) internally.

[StefanKarpinski]

How can CholeskyDense(A) indicate whether it worked or not? It's supposed to return an object of type CholeskyDense so it seems like there would have to be a notion of a CholeskyDense object that somehow encodes the fact that A wasn't positive definite?

[JeffBezanson]

I was thinking CholeskyDense could have a field that tells you, but the important thing is some low-level function that doesn't throw an error so you have the option of more control.

[andreasnoack]

@JeffBezanson wrote:
! convention:
after f!(A), A should equal B in B=f(A)

This is not the case in lapack.jl and I think it could be too restrictive. The LAPACK functions return tuples typically with a matrix and error information and for example syevr! just destroys A and returns eigenvalues. Otherwise many of the LAPACK functions should copy their input and have ! removed.

@StefanKarpinski CholeskyDense gives and error if the decomposition fails but LUDense and CholeskyDensePivoted don't. They just save the information variable returned from LAPACK. For the former it is used in the det method and the latter has a rank method but fails if the factorisation is used for solving a system of equations.

[JeffBezanson]

All done now, except for assign! which I did on a branch since it is very disruptive.