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

Add many "see also" links to docstrings #38606

Merged
merged 29 commits into from
Apr 19, 2021
Merged

Conversation

mcabbott
Copy link
Contributor

@mcabbott mcabbott commented Nov 29, 2020

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:

  • giving examples of floatmax and typemax under each other's headings,
  • showing similar under Array{T}(undef, ...)
  • mentioning that comprehensions use collect on various Iterators. objects.

Edit -- more:

  • correcting map and foreach to say "iteration stops when any iterator is finished" and map! to say "destination must be at least as large as the smallest collection" not the first.
  • adding examples to short-circuiting && etc.
  • adding examples like ℯ^(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.

@dkarrasch dkarrasch added the docs This change adds or pertains to documentation label Nov 29, 2020
@mcabbott
Copy link
Contributor Author

CI doesn't like this, I'm not sure why. Is there a trick needed for these?

┌ Warning: no doc found for reference '[`@info`](@ref)' in src/base/base.md.
┌ Warning: no doc found for reference '[`copy!`](@ref)' in src/base/base.md.
┌ Warning: no doc found for reference '[`⊉`](@ref)' in src/base/collections.md.

@vtjnash
Copy link
Member

vtjnash commented Nov 29, 2020

Possibly not the canonical name for it? Often they are [@ref Base.copy!] and such

@fredrikekre
Copy link
Member

Alternatively they are not included in the manual. For example @info does not exist, it is only documented as the generic @logmsg

@mcabbott
Copy link
Contributor Author

Thanks for the hints. I'm slowly ironing them out locally, a few more builds to go.

base/iterators.jl Outdated Show resolved Hide resolved
@mcabbott
Copy link
Contributor Author

mcabbott commented Dec 22, 2020

I couldn't make a reference to @logmsg or Logging.@logmsg, but could make a reference to Logging which might be narrow enough.

Questions:

  • Can I include references from Base to functions in standard libraries somehow, or is that impossible? At the moment I just reference the library itself, or the heading of the manual section. That's more useful for @printf than for LinearAlgebra.
  • Can I refer to labels like kw"?" for the ternary operator, or kw"&&", from other docstrings? I think I deleted my attempts from the present state of this.

@ViralBShah
Copy link
Member

It would be great to get this in.

@mcabbott
Copy link
Contributor Author

mcabbott commented Jan 21, 2021

Excellent, what needs to happen, do you think?

  • Some functions are newly documented, i.e. added to doc/src/base/.... Maybe that's a review priority, does this make them more official? I'm not so sure of the policy here.
  • I'm still unclear how to create some cross-links, if anyone can enlighten me on Add many "see also" links to docstrings #38606 (comment) I'd appreciate it.
  • More generally if anyone wants to scan a few of these and object to any (or any patterns) that would also be great, even if you don't feel like reading all of them.

@ViralBShah
Copy link
Member

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.

@mcabbott
Copy link
Contributor Author

mcabbott commented Jan 22, 2021

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 promote_typejoin) which seemed to deserve mention from somewhere (like promote_type) and cross-links without targets don't work. I have no opinions as to whether these functions are otherwise important enough to deserve documentation. If they don't, then the options are either to remove them entirely from "See also" to keep them hidden, or to mention them without making a hyperlink.

@ViralBShah
Copy link
Member

Makes sense.

base/permuteddimsarray.jl Outdated Show resolved Hide resolved
base/promotion.jl Outdated Show resolved Hide resolved
base/strings/io.jl Outdated Show resolved Hide resolved
base/tuple.jl Outdated Show resolved Hide resolved
@vtjnash
Copy link
Member

vtjnash commented Apr 7, 2021

Some comments of places to fix, but otherwise, lgtm

base/strings/io.jl Outdated Show resolved Hide resolved
Copy link
Member

@vtjnash vtjnash left a 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

@@ -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)
Copy link
Member

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

Copy link
Contributor Author

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?

@vtjnash vtjnash merged commit 2435f96 into JuliaLang:master Apr 19, 2021
@mcabbott mcabbott deleted the doc1 branch April 19, 2021 21:21
ElOceanografo pushed a commit to ElOceanografo/julia that referenced this pull request May 4, 2021
antoine-levitt pushed a commit to antoine-levitt/julia that referenced this pull request May 9, 2021
johanmon pushed a commit to johanmon/julia that referenced this pull request Jul 5, 2021
Keno pushed a commit that referenced this pull request Jun 5, 2024
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants