-
-
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
Add many "see also" links to docstrings #38606
Conversation
CI doesn't like this, I'm not sure why. Is there a trick needed for these?
|
Possibly not the canonical name for it? Often they are |
Alternatively they are not included in the manual. For example |
Thanks for the hints. I'm slowly ironing them out locally, a few more builds to go. |
I couldn't make a reference to Questions:
|
It would be great to get this in. |
Excellent, what needs to happen, do you think?
|
I would think that the newly documented ones should be in a separate PR than the cross-links. That might perhaps make it easier to get things merged rather than all in one shot. I wonder what others have to say about that. |
I don't mind moving them, although note that the two PRs will still be coupled, in the sense that links here won't work until their targets exist. The advantage of reviewing them in here is that you can easily search for the (proposed) links toward a given function. (They should all be gathered about here on the super-long "files changed" page.) They are just things (like |
Makes sense. |
Some comments of places to fix, but otherwise, lgtm |
Co-authored-by: Jameson Nash <[email protected]>
Co-authored-by: Jameson Nash <[email protected]>
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.
A couple nits, and we should merge this
Co-authored-by: Jameson Nash <[email protected]>
base/irrationals.jl
Outdated
@@ -19,7 +19,7 @@ for integers `n` will give a rational result when `n` is a perfect square), then | |||
abstract type AbstractIrrational <: Real end | |||
|
|||
""" | |||
Irrational{sym} <: AbstractIrrational | |||
`Irrational{sym}` <: [`AbstractIrrational`](@ref) |
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.
oh, I didn't necessarily mean we should do this now. I'm probably in favor, but we should let others respond first in a separate PR
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.
Sorry, now reverted.
Possibly it could be done at a higher level, too, just make all types in docstrings clickable somehow?
This adds many more cross-links to the functions' docstrings. They are a little random, but the goal is to link to a few similar or contrasting functions whose existence, or precise names, might not be known.
They are mostly above
# Examples
as this seemed to be the policy. Although perhaps if the lists get longer, they ought to move to below.I also edited a few other docstrings, including:
floatmax
andtypemax
under each other's headings,showingsimilar
underArray{T}(undef, ...)
collect
on variousIterators.
objects.Edit -- more:
map
andforeach
to say "iteration stops when any iterator is finished" andmap!
to say "destination
must be at least as large as the smallest collection" not the first.&&
etc.ℯ^(im)π ≈ -1
to irrational constants.I also added some functions to doc pages, to have something to refer to. Not sure how big a deal this is or isn't; I don't want to accidentally make policy.