Add diagm for vectors

[eulerkochy]

This in reference to JuliaLang/LinearAlgebra.jl#609 . I've added the missing function definition.

[StefanKarpinski]

Thanks for the contribution! This should also add a test for the method, checking that it works as expected as well as documentation and a NEWS entry.

[eulerkochy]

I've removed the un-necessary commit from the branch!

[StefanKarpinski]

At this point I would just do git fetch to pull the latest master and then git reset origin/master and edit the code until the diff looks the way you want it to. This is a small change and the git gymnastics required to do this via commits and rebasing aren't really worth it.

[eulerkochy]

Is it fine now?

[KristofferC]

Is it fine now?

If you look at https://github.com/JuliaLang/julia/pull/31125/files you can see that many irrelevant files seems to have been deleted in this PR. There is also still no test added.

[eulerkochy]

Tests as in documented examples of the function call? If so, what's the format to be followed?

[KristofferC]

Tests as in testing the functionality of the new feature (using the @test macro), see e.g. https://github.com/JuliaLang/julia/blob/master/CONTRIBUTING.md#writing-tests.

Also we could update all the instances of syntax like diagm(0 => D.diag) in the code to use diagm(D.diag) instead.

[eulerkochy]

Interesting! But can I make a separate PR for that? This PR has too clumsy for a lapse on my part and I'm not proud of this fact. :(

[KristofferC]

Sure, that can very well be in another PR.

[eulerkochy]

Ok, what's left to do in this PR? Can anyone tell? @KristofferC @StefanKarpinski

[pkofod]

Ok, what's left to do in this PR? Can anyone tell? @KristofferC @StefanKarpinski

No need to ping when people are already in the conversation :)

I did note the conversation above, but would it really be such a hassle to simply include the tests in this PR? It's easier to find at a later point if it's in the same PR, and you'll actually be 110% confident that your changes work as intended. Even the smallest changes can have unintended consequences, so it's just good practice IMO. I get the eagerness to have the PR merged, but honestly, there's not going to be a new release of Julia tomorrow (I don't think there is at least ! ) so if this is merged now or if it's merged three days from now WITH tests really doesn't matter from an end-users perspective - so I'd say the latter (wait and merge with tests) makes sense. Even for such a "simple" (though appreciated) PR.

[eulerkochy]

Thanks for the advice, and well I was just curious to know what remaining work was left so that I could complete it. Seems like to add tests for it!

[eulerkochy]

Just trying to reopen!

[StefanKarpinski]

Getting there!

[StefanKarpinski]

Looking good. Glad you stuck it out!