-
Notifications
You must be signed in to change notification settings - Fork 246
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
Migrate doc from Sphinx to Documenter #347
Conversation
Convert rst to md using https://gist.github.com/zaiste/77a946bbba73f5c4d33f3106a494e6cd Add Julia syntax highlighting to md files Add make.jl for Documenter.jl Autodeploy doc in .travis.yml Closes JuliaCollections#346
Pandoc have been used to convert reStructuredText to Markdown using: FILES=*.rst
for f in $FILES
do
filename="${f%.*}"
echo "Converting $f to $filename.md"
pandoc $f -f rst -t markdown -o $filename.md
done |
Codecov Report
@@ Coverage Diff @@
## master #347 +/- ##
=========================================
+ Coverage 96.45% 96.5% +0.04%
=========================================
Files 31 31
Lines 2286 2287 +1
=========================================
+ Hits 2205 2207 +2
+ Misses 81 80 -1
Continue to review full report at Codecov.
|
Definition list in doc/src/sorted_containers.md is not rendered correctly. Any idea how to fix this? |
because this is Documenter.jl's default
@scls19fr, thanks for doing this! I've been going through my messages in order, and finally caught up to this one (so most of my previous comments were out of date). Not sure I'll have more time today, but will try to take a closer look over the next couple of days. |
Approximately 2-3 years ago when I wrote the code for sorted containers, I switched all the DataStructures documentation from MD to RST by hand (with Kevin's permission) precisely because I couldn't figure out how to make definition lists in MD, and I needed definition lists for the sorted containers documentation. |
Fashion is an eternal renewal ;-) |
Okay, had a look. :-) Regarding Would you be up to doing that here? It should probably be done for all methods/types, but since sorted containers are the most problematic, I think just doing it there for now would be fine. The rest can be moved gradually--e.g., as the package is being split up (cc: @oxinabox). |
Just a remark that some of the functions in the sorted container function list apply to more than one of the sorted containers and hence correspond to more than one location in the source code. |
I moved most definitions in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this looks good! I made a few minor requests for changes.
src/sorted_dict.jl
Outdated
SortedDict(o, k1=>v1, k2=>v2, ...) | ||
|
||
Construct a `SortedDict` from the given pairs with the specified | ||
ordering `o`. If `K` and `V` are not specified, the key type and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
K
and V
don't appear here.
`key=>value` pairs. If `K` and `V` are not specified, the key type | ||
and value type are inferred from the given iterable. The ordering | ||
object `o` defaults to `Forward`. | ||
""" | ||
SortedDict{K,D}(kv) where {K,D} = SortedDict{K,D}(Forward, kv) | ||
function SortedDict{K,D}(o::Ord, kv) where {K,D,Ord<:Ordering} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function signature here doesn't match the one in the comments above. Might need to rearrange the definitions a little.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem is is similar to #340 and should probably be adressed in an other PR
src/sorted_dict.jl
Outdated
to `Forward`. | ||
|
||
See below for more information about ordering. | ||
""" | ||
SortedDict(kv, o::Ordering=Forward) = SortedDict(o, kv) | ||
function SortedDict(o::Ordering, kv) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
src/sorted_set.jl
Outdated
@@ -5,6 +5,19 @@ | |||
mutable struct SortedSet{K, Ord <: Ordering} | |||
bt::BalancedTree23{K,Void,Ord} | |||
|
|||
""" | |||
SortedSet(iter, o=Forward) | |||
and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I personally think that the "and" lines are not needed (here and elsewhere).
I've just add references to docstrings into sorted_containers.md about "and" lines... that's difficult to avoid them (at least in this PR) Some constructors's docstring will probably have to be modified... but I think it's safer to adress this problem in an other PR. |
This looks good--as you suggested, I'm okay with fixing the remaining issues in future pull requests. However, there are merge conflicts now. Are you comfortable fixing these? I can give some command-line help if you need it. |
My git fu is a bit rusty... So some help to deal with this 2 conflicting files could help. |
Solving this merge conflict wasn't easy for me (and I don't like this kind of task)... but I think it's now ready for merging. |
Current doc is available at ReadTheDocs http://datastructuresjl.readthedocs.io/en/latest I wonder if it was manually uploaded or automatically uploaded when commits are made. Documenter.jl provides a nice doc about hosting on github pages https://juliadocs.github.io/Documenter.jl/stable/man/hosting/ I wonder if we should move out of RTD to GH-pages or not... and who can do it? If some improvements to this PR are required please tell me. |
Should we merge this before #348? |
Sorry, been swamped recently, but I plan to look at this tomorrow. |
Any news about this PR? Is there any problems on which I could help? I noticed that some recent PR involve a lot of changes in several files. I would prefer to avoid to have to rebase this PR. |
Again, swamped (and sick right now). Is anyone else available to
|
I will try and hit this up in the next few days. What I want to do is branch this branch over to my own repo, |
Sorry, there were reasons, but anyway I am doing my review right now. |
It seems to be working but I get a couple of errors, that I can't quiet workout why (I think you mentioned them before)
I'm not sure if this is a blocking or not. I am going to push a few changes over to this branch shortly from my staging branch. You can see my staging site at: Others can check if that is good, it will stay hosted for a while. |
(Was blank on sphinx anyway)
After being merged, this needs the deploy key added to travis and github. People should take a look at the staging site: |
http://white.ucc.asn.au/DataStructures.jl/latest/disjoint_sets.html maybe comments (starting with Same for http://white.ucc.asn.au/DataStructures.jl/latest/accumulators.html (see On http://white.ucc.asn.au/DataStructures.jl/latest/default_dict.html |
http://white.ucc.asn.au/DataStructures.jl/latest/trie.html Being previously a big fan of Python and PEP8 I will change t=Trie{Int}() to t = Trie{Int}() same for others affectations t["Rob"]=42 -> t["Rob"] = 42 same for t["Roger"]=24 -> t["Roger"] = 24 I will add a space after coma, and two spaces before haskey(t,"Rob") #true -> haskey(t, "Rob") # true Same for get(t,"Rob",nothing) #42 -> get(t, "Rob", nothing) # 42 but these cosmetic changes could be tackled later if you prefer. |
Do it now, or do it later, I don't mind. |
I have now looked over the output and am satisfied nothing has gone critically wrong. |
Ok so let's publish it now under https://juliacollections.github.io/DataStructures.jl/latest and fix remaining issues later. |
I've added the hooks so that when I (or someone else) merges this tomorrowish, then the docs should just build and deploy. |
Ok, Done 🎉 |
Yes, I just clicked merge, and 5 minutes later (once travis run) new docs appeared at the new site. |
Thanks @oxinabox for merging |
Convert rst to md using https://gist.github.com/zaiste/77a946bbba73f5c4d33f3106a494e6cd
Add Julia syntax highlighting to md files
Add make.jl for Documenter.jl
Autodeploy doc in .travis.yml
Closes #346