inline documentation #182
inline documentation #182
Conversation
src/accumulator.jl
Outdated
| counter{T}(dct::Dict{T,Int}) = Accumulator{T,Int}(copy(dct)) | ||
|
|
||
| """ | ||
| ``` |
There was a problem hiding this comment.
I haven't looked at the rest, but the ` are not needed. OTOH, add 4 spaces before code lines (such as signatures).
There was a problem hiding this comment.
The convention is four spaces for the signature code block. Other code blocks can use `s.
There was a problem hiding this comment.
Might be useful to point at an example, so here is one I was just digging through:
https://github.com/JuliaLang/julia/blob/master/base/sharedarray.jl#L38-L51
|
@abhijithch, thanks for doing this! Not sure I'll have the chance to look soon, but hopefully you'll get a review and merge soon. @hayd, can you take a look? |
src/deque.jl
Outdated
| ### Args: | ||
| * `q`: A deque type datastructure. | ||
| Returns the first element of the deque `s`. |
There was a problem hiding this comment.
Should this be "Returns the first element of the deque q."?
|
Hopefully some constructive comments there. Thanks for the PR! |
|
Thanks @DanielArndt for the comments, will update the PR. |
|
@nalimilan Is there any standard best practices for documentation available anywhere? Its just that I saw the tick in another package and assumed that would be the right way, anyways will change it to four spaces. Thanks. |
src/accumulator.jl
Outdated
| """ | ||
| counter{T}(seq::AbstractArray{T}) | ||
| ### Args: |
There was a problem hiding this comment.
As I noted on the StatsBase PR, I think in most cases the argument list isn't useful. Here, seq is mentioned below anyway, and the signature gives the expected type.
|
@abhijithch Unfortunately, I don't know whether there's a style guide yet as regards the docs. I'm mostly taking example on what's in Julia Base. |
|
I've just made a PR to try defining guidelines: JuliaLang/julia#15136 |
abhijithch
force-pushed
the
inlinedoc
branch
from
4d8f5e6 to
d30f461
Compare
src/ordereddict.jl
Outdated
| """ | ||
| OrderedDict | ||
| 'OrderedDict's are simply dictionaries whose entries have a particular order. The order refers to insertion order, which allows deterministic iteration over the dictionary or set. |
There was a problem hiding this comment.
I think the intention here was to have ticks () instead of apostrophes (') so that OrderedDict` is rendered as code/monospace
There was a problem hiding this comment.
This should probably be broken into lines of 80 92 chars max
|
Just a few more comments. This is looking really good! |
src/queue.jl
Outdated
| """ | ||
| Queue(ty{T}[, blksize::Integer]) | ||
| This is a constructor to create an object of a `Queue`.The block size `blksize` by default is `1024`. |
There was a problem hiding this comment.
Instead of "The block size blksize by default is 1024.", just write blksize::Integer=1024 in the signature.
src/queue.jl
Outdated
| """ | ||
| dequeue!(s::Queue) | ||
| Removes an element from the back of the queue `s`. |
There was a problem hiding this comment.
Similarly to how enqueue! was flipped, so is this.
dequeue! should (and does) remove an item from the front of the queue
doc fixes minor doc fix
|
@nalimilan @DanielArndt @abhijithch should we merge this in? |
|
It would be good if the commits were squashed, although that's probably less necessary with documentation commits (unless they break something). |
|
Sorry, this hadn't fallen off my radar, I've just been super overwhelmed lately. I wanted to give this the solid review it deserves instead of trickling in comments. I think this can be merged as is. At some point I think it would be worth taking another stab at the compare documentation. It doesn't seem to follow the same tone as the other function docs, but I've been unsuccessful at rewording it so far. Thanks a lot @abhijithch -- and sorry I slowly trickled in comments. I hope that wasn't too frustrating. |
src/queue.jl
Outdated
| """ | ||
| Queue(T[, blksize::Integer=1024]) | ||
| This is a constructor to create an object of a `Queue`.`T` specifies |
There was a problem hiding this comment.
Could simply be "Create a Queue objet containing elements of type T."
|
Thanks, this looks good to me after the handful of small points I commented on are fixed. Squashing is a good idea too. |
|
Will update based on @nalimilan 's comments. Thanks a lot for your guidance. |
7 participants
Basic inline documentation of the exported functions.