-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Doc hard, and with a vengeance #11943
Conversation
bb25f66
to
66a5b0f
Compare
This looks lossless so far! 👍 How do you want to handle RST specific features? Interpolating Julia code, maybe? e.g. |
Thank you for trying again. I'll give this more careful scrutiny tomorrow, but can you explain what the idea is in this current iteration? For example, are the lines following .. function:: test(pkgs...)
``test()``
Run the tests for all installed packages ensuring that each package's test dependencies are installed for the duration of the test. A package is tested by running its ``test/runtests.jl`` file and test dependencies are specified in ``test/REQUIRE``.
``test(pkgs...)``
Run the tests for each package in ``pkgs`` ensuring that each package's test dependencies are installed for the duration of the test. A package is tested by running its ``test/runtests.jl`` file and test dependencies are specified in ``test/REQUIRE``. |
A question for later design, maybe: I was discussing further with @jakebolewski the idea we floated offline about a second pass to detect cross-referrable entities within the docs. @jakebolewski reminded me that the help database currently isn't reified until the first help query is run. Does this mean that cross-references can only be generated after help is called once? Or will there need to be a second pass to generate rendered docstrings with cross-references statically? If so, how will runtime extensibility from loading new modules work? |
@one-more-minute, I'm glad you're taking another crack at this even after I got you into hot water the last time around. |
@StefanKarpinski No problem! Looking forward to this being done and dusted though. @jiahao So to try and make it clear what's going on here: In the previous system, the *.rst files were the primary source for docstrings, and they were loaded into Julia as a second step. This PR flips that around – docs/helpdb.jl is now the primary source for docstrings like That means that once this is in, the old help system will be entirely redundant – its mechanics aren't really relevant to us here. Things like cross references are worth discussing – I think we can make them a lot nicer than they were before, and it can all be done in a dynamic way. However, since this flip-around step is (hopefully) the most painful part of the process, I think it makes sense to make it as simple as possible, focus on moving without information loss, and build on it from there – deleting the help system, translating to markdown, implementing cross-references etc. etc., will be much easier if we can build them on top of the reorganised system in future PRs. It's a little unfortunate that the docs have to get worse before they get better, and obviously I'd love to do this stuff all in one go, but I just don't see a route that involves a reasonable level of effort. I think the very short-term pain will be well worth it long run. (Although arguably, this actually improves the REPL experience, since we can view more than one method of a function now.) |
Thanks Mike, that explanation was very helpful. I've examined the troublesome cases from the previous incarnation of this PR and they all look much better than before. Separating the issues of migrating from manual to docstring, from ReST conversion, is much clearer and avoids all the formatting weirdness like changing backquoting to double quotes. There is an issue with docstrings shared across multiple functions, e.g. There's also a minor formatting problem in the docstring itself, where only the first method is double-backquoted but other methods are not. That can be fixed easily though. |
Ok great, thanks for the feedback. Would it be sufficient to indent all signatures, i.e. changing that to
for them all to be properly formatted as code? We can handle shared docstrings reasonably nicely already by doing: @doc (@doc foo) bar It might be useful, and would be reasonably easy to implement, to have proper aliases for metadata. It might make sense to handle these cases as part of this translation – I'll play around with it. |
Since I think a lot of cases of Unicode operators have (or should have) an ASCII equalivalent operator or function name, it does seem like it would be good to make that sharing handled. |
Indentation would suffice in ReST when prefaced with a line containing only "::" |
Actually, unicode aliases like |
I'm pretty sure I've seen cases where there was just a Unicode character, and no ASCII alias |
The most complicated example of shared docstrings is Not all of these are exact synonyms. Can this case be handled? (If not, one serious alternative is to document only |
e5798c3
to
c742260
Compare
:: | ||
eye(A) | ||
|
||
Constructs an identity matrix of the same dimensions and type as ``A``. |
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.
eye(A)
is repeated 3 times.
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.
Yes, this is the methods issue mentioned in the PR description – three methods are listed in the manual, the methods are combined for the whole function, each method in the manual gets the docs for the whole function.
In cases like this we can probably just delete the second two method listings.
@jiahao Absolutely, we can handle those cases easily enough. Although if they're not actually synonyms it probably isn't a bad idea to give them separate docstrings with reference to each other. |
How does this look now? I've edited the head of the PR to look at the issues that aren't addressed in the code itself, but let me know if I've missed any feedback. |
@@ -788,13 +907,11 @@ Mathematical Functions | |||
|
|||
.. function:: trunc([T,] x, [digits, [base]]) | |||
|
|||
``trunc(x)`` returns the nearest integral value of the same type as ``x`` whose absolute |
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.
two methods for trunc
getting clobbered by the trunc
method in Dates
?
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.
Yes, that looks like it's the same issue as with eps
.
What's the status of this? Anything I or anyone else can do to help? Let's get an RC out! |
As long as there are no more major info losses that we can deal with systematically, I think this should basically be good to go (modulo a rebase). Once it's in I can work on what can be converted automatically, and we can crowdsource moving things around and fixing the remaining issues. I didn't get round to doing @MichaelHatherly's idea of splitting up the huge |
+1 for getting this merged and crowdsourcing the needed doc touch-ups. |
+1 to that |
a355ec8
to
b980d09
Compare
@@ -200,6 +204,7 @@ Macro reference | |||
would generate:: | |||
|
|||
if i_1 > size(A, 1) | |||
|
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.
Odd. Bug maybe?
Ok, we're doing this and going to deal with various issues that need to be fixed as they come up. |
Doc hard, and with a vengeance
Woo hoo! Thanks @one-more-minute, @MichaelHatherly et al... |
Yeah, probably the right thing to do. Would still be nice to get answers to the honest questions I had above. Hard to fix known issues if no one else knows how to go about fixing them. And we'll have to be very careful about open PR's that touch parts of the docs that are now autogenerated from docstrings. |
I agree, but we needed to move on this and get a 0.4 RC out the door. |
Sure, my 2 small "before merging" bullet items were taken care of. Now we all need to clean this up and let everyone who has an open PR or will file any future PRs touching docs know what they need to do. I still don't get where or what needs to be done to restore the data that's currently in helpdb but not in the rst docs, do you? As far as @ViralBShah's idea above goes, I was actually thinking the other way around. This PR created the docstrings already so now we should hopefully be free to move them around however we want, but conversion from rst to markdown and moving away from sphinx seems way premature and not something we should try to do while we're worrying about release candidates. |
All open PR's that touch docs (likely an overestimate): #12432 dumb code I used to get that list, feel free to make fancier
|
So what's the next step now? I read the comments but I'm still not sure. Is it to transfer the stuff from |
👍 💯 for doing this! Now to iron out any glitches before 0.4.0 release. help?> convert(UTF32String, Char['a','b','c'])
help?> convert(UTF8String, Char['a','b','c'])
help?> convert(UTF8String, UInt8[64,65,66])
help?> convert(Int, 3.0) Old: help?> convert(Int, 3.0)
convert(::Type{Int64}, x::Float64) at int.jl:205
julia> convert(UTF8String, UInt8[64, 65, 66])
"@AB"
help?> convert(UTF8String, UInt8[64, 65, 66])
convert(::Type{UTF8String}, dat::Array{UInt8,1}) at unicode/utf8.jl:231 |
That looks like a regression that is probably worth a separate issue. |
Echoing @KristofferC , what is the next step for now? If changes are not expected to be made to |
Docstring changes are fine, but they should be made to The next step is to convert stuff to markdown, #12435, and once that's in we can all start to move the docstrings to appropriate places. |
Don't know if you missed this or are ignoring me, but again, would be great to get answers for the following
|
Sure, have just had a lot on so things might've slipped by. Just moving the docstrings above the correct methods will give them the correct type info, yes – all that stuff's already in place. Then we just need a syntax for type info in the RST docs and for genstdlib.jl to recognise that syntax, e.g. we'd change
to
so that the specific method doc is spliced in instead of the whole function. |
Awesome. I'll experiment with moving around the functions getting clobbered by Dates module docs, and Some of the RST docs have already been using declaration-like type syntax, but not all. |
Ok, great – it also occurs to me that I can actually just run the #12435 conversion over all of Base if necessary, so it's not really a big deal if we start moving docstrings before then. |
Well, tried https://gist.github.com/b5c7657d53d5c37b4e85 but so far that doesn't appear to be helping much. And at the repl this might just be #12437?
I think the top priority needs to be getting the repl help usable and fixing the manual back up, more than worrying about what format it's in. |
Alright, so for some reason I've come back for round two. This time I hope that what's going on is a lot clearer, and it's more automated so that it'll be easier to wait for input from whomever wants to provide it.
Remaining issues:
eps
as separate fromDates.eps
, but they're actually the same function and so one clobbers the other.∉
and∋
within
.I think there are few enough of these cases (aside from the methods thing) that we can fix them by hand later.
From a quick check, a relatively small number (222) of the docstrings use weird
:...:
syntaxes which means the rest (1036) should be easy to convert to markdown automatically.