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

Improve docs for sin and some friends #45137

Merged
merged 8 commits into from
Jul 13, 2022
Merged

Conversation

mcabbott
Copy link
Contributor

@mcabbott mcabbott commented May 1, 2022

Some "see also" links from #38606 were missing (@ref) and thus not really links.

Also adds a few more such links, and adds examples for sin and a few other functions.

base/math.jl Outdated
# Examples

```jldoctest
julia> tanh.(-3:3f0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using a Float32 literal here does not seem the most beginner friendly to me. 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's mostly just a hack to avoid printing 16 digits here.

But also, strikes me as easy to ignore if you don't know, and something which you eventually should know. If you paste that bit into the REPL to see, then I guess you don't learn much:

julia> -3:3f0
-3.0f0:1.0f0:3.0f0

Maybe it could be clarified in some other way?

Copy link
Contributor

@CameronBieganek CameronBieganek May 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be better to use the explicit round broadcasting like you did for sin. Or else just live with the 16 digits of printing. Doing the rounding feels kind of like hiding the real output anyways. (Though the rounding looks good for the sin example.)

base/math.jl Outdated Show resolved Hide resolved
See also [`@irrational`], [`AbstractIrrational`](@ref).
See also [`@irrational`](@ref Base.@irrational), [`AbstractIrrational`](@ref).
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly this was left off last time to avoid this error:

[ Info: CrossReferences: building cross-references.
��� Error: no doc found for reference '[`Base.@irrational`](@ref)' in src/base/numbers.md.
��� @ Documenter.CrossReferences /buildworker/worker/doctest_linux64/build/doc/deps/packages/Documenter/HmHje/src/Utilities/Utilities.jl:32
[ Info: CheckDocument: running document checks.
[ Info: Populate: populating indices.
ERROR: LoadError: `makedocs` encountered an error. Terminating build

Should this be documented?

@CameronBieganek
Copy link
Contributor

It looks like this works:

julia> for f in (:foo, :bar)
           @eval begin
               "This is the $(nameof($f)) function."
               $f(x) = 2x
           end
       end

help?> foo
search: foo floor pointer_from_objref OverflowError RoundFromZero unsafe_copyto! functionloc StackOverflowError @functionloc OutOfMemoryError

  This is the foo function.

help?> bar
search: bar baremodule SubArray GlobalRef clipboard BitArray backtrace BitMatrix catch_backtrace AbstractRange AbstractArray AbstractIrrational

  This is the bar function.

So we could use that to add foo(::Missing) docstrings for all the math functions here and here.

@mcabbott
Copy link
Contributor Author

mcabbott commented May 1, 2022

Maybe make an issue if you think all missing methods should be documented that way. As I said above, this seems very noisy to me, and sure to be incomplete.

@CameronBieganek
Copy link
Contributor

CameronBieganek commented May 1, 2022

Ok, in that case I think you should remove the Number annotation from sin(x::Number) in the docstring. If you leave that annotation there, then there are the following two docstrings for sin:

  • sin(x::Number)
  • sin(A::AbstractMatrix)

That implies that sin(missing) should throw a MethodError.

EDIT: Or you could change the annotation to sin(x::Union{Missing, Number}.

@KristofferC
Copy link
Member

I think it is worth being a bit more careful about not introducing too much new vertical space unless it is really really needed. Doing ? function and then getting two pages of examples is usually kind of annoying because you have to scroll up so far to see what the function actually does. So if long examples are crucially needed to make a point about some specific behavior of the function then so be it but it should at least be given some careful thought.

@dkarrasch dkarrasch added the docs This change adds or pertains to documentation label May 2, 2022
@oscardssmith
Copy link
Member

this is one of many reasons why it would be good to make help mode page.

@mcabbott
Copy link
Contributor Author

mcabbott commented May 2, 2022

Yes to scrolling too much being a pain.

I guess this PR is feeling the waters for making many docstrings more detailed. Inspired by this Discourse thread and one before, it seems that many people would appreciate more details, more tangentially related examples, as an informal way to learn what's possible and how to do it.

For inspiration, Mathematica's page is pretty great. That's probably impossible without a team of professionals, but small steps towards that still seem valuable. They have 2 levels of section headings (and a separate window) to avoid the scrolling problem.

Is there an issue about making help paged, like less? There is # Extended help (not exactly sure when this was added) but it doesn't seem like the ideal mechanism.

@johnmyleswhite
Copy link
Member

To add to @mcabbott's point, it would be good to get a consistent direction for function documentation set -- will it (a) gradually expand (as many users prefer, especially new users) or (b) will it always stay as pithy as it is today? Unless there's a clear commitment to expansion soon, it may be advantageous to built an alternative documentation store for a while that either eventually gets ported over or lives forever as a separate resource. That way people can make progress without being blocked on waiting for an official strategy.

@giordano
Copy link
Contributor

giordano commented May 2, 2022

Would it make sense to have help mode ask for which method to show the docstring, if there is more than one (which an option to show all of them)?

@ViralBShah
Copy link
Member

In my mind there is no doubt that we need to allow for the documentation to expand substantially, and implement whatever tooling we need in the docsystem. Until then, people just have to scroll. But keeping it short so you don't have to scroll feels wrong to me.

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

Do we want to do tihs?

@ViralBShah ViralBShah merged commit c1750f2 into JuliaLang:master Jul 13, 2022
@mcabbott mcabbott deleted the doc4 branch July 13, 2022 03:23
ffucci pushed a commit to ffucci/julia that referenced this pull request Aug 11, 2022
* missing ref in sin,cos,log

* a few links

* a few examples

* irrational + promote_type had the same problem
pcjentsch pushed a commit to pcjentsch/julia that referenced this pull request Aug 18, 2022
* missing ref in sin,cos,log

* a few links

* a few examples

* irrational + promote_type had the same problem
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.

8 participants