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

RFC: Add documentation for givens, ldltfact!, and eigvals!. Fixes #10526 #12852

Merged
merged 1 commit into from
Sep 21, 2015

Conversation

andreasnoack
Copy link
Member

I think this shouldn't be merged before the other doc changes are completed.

Please check for bad language, typos and errors.

@kshyatt kshyatt added docs This change adds or pertains to documentation linear algebra Linear algebra labels Aug 28, 2015

such that `G*A[:,col]=y` with `y[i1]=r`, and `y[i2]=0`. The cosine and sine of the rotation angle can be extracted from the `Givens` type with `G.c` and `G.s` respectively. The arguments `f` and `g` can be either `Float32`, `Float64`, `Complex{Float32}`, or `Complex{Float64}`. The `Givens` type supports left multiplication `G*A` and conjugated transpose right multiplication `A*G'`. The type doesn't have a `size` and can therefore be multiplied with matrices of arbitrary size as long as `i2<=size(A,2)` for `G*A` or `i2<=Size(A,1)` for `A*G'`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i2<=Size(A,1) should be size, right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same above.

@kshyatt
Copy link
Contributor

kshyatt commented Sep 6, 2015

Now that #12835 is merged, is this ready to go?

@yuyichao
Copy link
Contributor

yuyichao commented Sep 6, 2015

If these are public doc, they should follow the format of other doc in helpdb.jl and add the function signature to the relavant section in the rst doc

@ViralBShah ViralBShah added this to the 0.4.0 milestone Sep 7, 2015
@andreasnoack
Copy link
Member Author

@yuyichao Is this process described anywhere? I don't really understand the new system. Would you explain "..add the function signature to the relavant section in the rst doc"?

@KristofferC
Copy link
Member

The documentation strings should look like this, where the method signature is intented as the line in the doc string:

doc"""
    chol(A, [LU]) -> F

Compute the Cholesky factorization of a symmetric positive definite matrix `A`
and return the matrix `F`. If `LU` is `Val{:U}` (Upper), `F` is of type `UpperTriangular`
and `A = F'*F`. If `LU` is `Val{:L}` (Lower), `F` is of type `LowerTriangular` and 
`A = F*F'`. `LU` defaults to `Val{:U}`.
"""

In the .rst file where the doc should go there should be a corresponding entry, like:

.. function:: chol(A, [LU]) -> F

The documentation will then automatically be inserted in the .rst-file when genstdlib.jl is run.

@andreasnoack
Copy link
Member Author

@KristofferC Thanks. We better write that down somewhere at some point.

@yuyichao
Copy link
Contributor

yuyichao commented Sep 7, 2015

I thought @jakebolewski put it in CONTRIBUTE.md but I guess it doesn't include the part about adding new function docs. Since I don't see that the genstdlib hack is going away anytime soon, I agree it's probably better to add this too...

@JeffBezanson
Copy link
Member

This is one of 2 remaining 0.4 issues. Is this release blocking? Please merge or remove the milestone as soon as practical.

@andreasnoack andreasnoack modified the milestones: 0.4.x, 0.4.0 Sep 8, 2015
@andreasnoack andreasnoack changed the title WIP: Add documentation for givens, ldltfact!, and eigvals!. Fixes #10526 RFC: Add documentation for givens, ldltfact!, and eigvals!. Fixes #10526 Sep 18, 2015
@andreasnoack
Copy link
Member Author

Okay. I think this one is ready for review now. To counter comments on the lack of hard newlines: I still think new lines should be soft because

  1. it allows users to have the terminal width that suits them without getting an ugly combination hard and soft new lines
  2. you can edit the text without adjust all lines in a paragraph.
  3. markup features result in a different line lenght in source and rendered text anyway

If there is now a consensus for hard newlines and if the parsing of newlines works then I'll of course update the pr.

@KristofferC
Copy link
Member

A new line in the doc string doesn't count as a hard newline does it? I thought markdown didn't care about single new lines. Does the in-terminal-help hard break the line on single new lines in doc strings?

@tkelman
Copy link
Contributor

tkelman commented Sep 18, 2015

Github refuses to wrap long lines in julia code. Until that changes we should wrap long lines for review and readability.

@andreasnoack
Copy link
Member Author

@KristofferC you are right. So it is only a matter of formatting the source.

@tkelman I don't wan't this pr to take much longer so I've hit enter a couple of times in the source. However, I've just realized that GitHub's split mode actually wraps lines so if you switch to that you'll probably see a nice altenation between soft and hard newlines where every second line has one word.

andreasnoack added a commit that referenced this pull request Sep 21, 2015
RFC: Add documentation for givens, ldltfact!, and eigvals!. Fixes #10526
@andreasnoack andreasnoack merged commit c8d5804 into master Sep 21, 2015
@andreasnoack andreasnoack deleted the anj/givensdoc branch September 21, 2015 15:19
andreasnoack added a commit that referenced this pull request Sep 28, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This change adds or pertains to documentation linear algebra Linear algebra
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants