Move docs inline from helpdb/Base.jl #19674
Conversation
|
Overall nice work here! I'm not sure that docstrings get applied properly when not directly atop a function though, so when you chain docstrings like you have in reduce.jl, I don't think that will work as intended. What you could do there instead is mirror the structure of helpdb, wherein each function is listed bare with its docstring above, e.g. """
sum(x)
Take the sum or whatever
"""
sum |
xorJane
force-pushed
the
MoveInline_122116
branch
5 times, most recently
from
69c7554
to
0eb1f47
Compare
| 7 | ||
| ``` | ||
| """ | ||
| function sum(A, dims) end |
There was a problem hiding this comment.
If I'm not mistaken, this should just be sum, otherwise it adds/overwrites methods. (Similarly for the other functions in here)
There was a problem hiding this comment.
Thank you for the feedback on both counts! I'll try that. I realized some of my updates were breaking things but I hadn't figured out what I was doing wrong yet.
There was a problem hiding this comment.
My pleasure. Great job here so far!
There was a problem hiding this comment.
Yes, should just be sum(A, dims), or whatever signature is most suitable.
There was a problem hiding this comment.
Updated function signatures as discussed. Thank you!
|
Needs a rebase, but this is great @xorJane |
| julia> isinteger([1; 2; 5.5]) | ||
| false | ||
| ``` | ||
| """ |
There was a problem hiding this comment.
This docstring is more general than the signature z::Complex so should probably be moved to a more suitable method definition if one exists.
(Same thing applies to a couple of others below this.)
There was a problem hiding this comment.
I moved the docstring for isinteger() to number.jl where isinteger() is defined for x::Integer. I decided to move most of the other docstrings that I had previously placed in complex.jl to method definitions for Real numbers in the appropriate files.
|
|
||
| Exponentiation operator. | ||
| """ | ||
| Base.:(^)(x, y) |
There was a problem hiding this comment.
Was Base.:(^)(x, y) meant to be included here, or just a copy-paste error?
There was a problem hiding this comment.
Adding that line was intentional. Without the line Base.:(^)(x, y), moving the docstring for exponentiation inline causes Documenter's output to include the message !! No docs found for 'Base.:^(::Any, ::Any)'. [src/stdlib/math.md].
Do you know if this message suggests a real problem? You can get the docstring for exponentiation at the REPL in any case.
Thanks!
There was a problem hiding this comment.
Do you know if this message suggests a real problem?
Yes, warnings from Documenter should always point to an actual problem. #19701 will turn these warnings into errors.
The best way I've found to locate where to move docstrings to is to use methods(func) and go through the list to see which is the most general definition that applies to the docstring. From the looks of the possible methods I'd say
^(x::Number, y::Number) in Base at promotion.jl:194
is probably the best choice. So the way to avoid the No docs found error is to move the docstring to that definition in promotion.jl and then change the signature listed in doc/src/stdlib/math.md with the following diff
diff --git a/doc/src/stdlib/math.md b/doc/src/stdlib/math.md
index e55967911..98d545190 100644
--- a/doc/src/stdlib/math.md
+++ b/doc/src/stdlib/math.md
@@ -9,7 +9,7 @@ Base.:-(::Any, ::Any)
Base.:*(::Any, ::Any...)
Base.:(/)
Base.:\(::Any, ::Any)
-Base.:^(::Any, ::Any)
+Base.:^(::Number, ::Number)
Base.fma
Base.muladd
Base.divThen just repeat the same steps for the other docs that are being moved.
You can get the docstring for exponentiation at the REPL in any case.
Yes, the REPL help mode isn't as strict about what kind of results it returns; it'll return all possible signatures that match rather than just an exact match. Documenter is just a bit more picky to make it easier to select a specific signature without including all the others that are less specific.
There was a problem hiding this comment.
This was a fantastic explanation! Thanks a lot. I updated accordingly and am no longer getting any warnings from Documenter.
| 7 | ||
| ``` | ||
| """ | ||
| function sum(A, dims) end |
There was a problem hiding this comment.
Yes, should just be sum(A, dims), or whatever signature is most suitable.
xorJane
force-pushed
the
MoveInline_122116
branch
4 times, most recently
from
7e570d4
to
561902e
Compare
|
|
||
| The largest `a^n` not greater than `x`, where `n` is a non-negative integer. | ||
| `a` must be greater than 1, and `x` must not be less than 1. | ||
| """ | ||
| # largest a^n <= x, with integer n |
There was a problem hiding this comment.
delete the comment, now redundant
| losslessly, some loss is tolerated; for example, `promote_type(Int64,Float64)` returns | ||
| `Float64` even though strictly, not all `Int64` values can be represented exactly as | ||
| `Float64` values. | ||
| """ | ||
| # Try promote_rule in both orders. Typically only one is defined, | ||
| # and there is a fallback returning Bottom below, so the common case is |
There was a problem hiding this comment.
move this comment block inside the function?
There was a problem hiding this comment.
Done. Thanks for reviewing!
xorJane
force-pushed
the
MoveInline_122116
branch
2 times, most recently
from
3078b76
to
0ce2bca
Compare
| """ | ||
| isinteger(x) -> Bool | ||
|
|
||
| Test whether `x` or all its elements are numerically equal to some integer. |
There was a problem hiding this comment.
is the array version of this slated to be deprecated? if so this docstring should be adjusted along with it
There was a problem hiding this comment.
isinteger(x::AbstractArray) returns a single Bool rather than operating elementwise, so it's not among the vectorized functions slated for deprecation in favor of compact broadcast syntax. Are you suggesting deprecation in favor of all(isinteger, x) separate from the devectorization deprecations? Best!
There was a problem hiding this comment.
Does seem unnecessary/redundant and a bit confusing to have that method now that all(isinteger, x) should be just as fast. Guess this case has more in common with maxabs etc than it does with the vectorized functions.
There was a problem hiding this comment.
Agreed, perhaps post in #19598 for feedback?
|
|
||
| Cumulative sum of `A` along a dimension, storing the result in `B`. The dimension defaults | ||
| to 1. | ||
| """ |
There was a problem hiding this comment.
probably should attach this to the more general method cumsum!(B, A, axis::Integer=1) = accumulate!(+, B, A, axis) below instead of one specific to v::AbstractVector?
There was a problem hiding this comment.
Great, moved it. Thank you!
| true | ||
| false | ||
| ``` | ||
| """ |
There was a problem hiding this comment.
should this be attached to a more general signature rather than a BitArray-specific one?
There was a problem hiding this comment.
Sorry for the delay! Do you think attaching this docstring to xor(x::Bool, y::Bool) in bool.jl is reasonable?
There was a problem hiding this comment.
Cheers. Just moved it.
xorJane
force-pushed
the
MoveInline_122116
branch
from
0ce2bca
to
077746b
Compare
xorJane
force-pushed
the
MoveInline_122116
branch
from
077746b
to
61cecee
Compare
|
Very nice @xorJane, thank you! |
This PR moves some docstrings inline from helpdb. Thanks in advance for reviewing!