-
-
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 documentation for [Abstract]Vector/[Abstract]Matrix #22674
Conversation
base/array.jl
Outdated
const AbstractVector{T} = AbstractArray{T,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.
A line break before this would be good for readability.
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.
Done.
@@ -877,7 +878,7 @@ end | |||
getindex(A, inds...) | |||
|
|||
Return a subset of array `A` as specified by `inds`, where each `ind` may be an | |||
`Int`, a `Range`, or a `Vector`. See the manual section on | |||
`Int`, a `Range`, or a [`Vector`](@ref). See the manual section on |
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.
Perhaps Range
should also get a ref
? (Int
is probably fine without one.)
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.
Range
does not have a docstring, so leaving that one out.
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.
Ah, didn't realize. That's too bad.
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.
Perhaps documenting Range
deserves an intro issue?
base/abstractarray.jl
Outdated
|
||
Abstract array supertype which arrays inherit from. | ||
Abstract `N`-dimensional array supertype with elements of type `T` | ||
which arrays inherit from. |
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.
I know this is how it was before, but personally I find this wording quite awkward. Perhaps it would be worth clarifying it a bit while you're in here? Maybe something like
Abstract
N
-dimensional array supertype containing elements of typeT
.Array
s and some array-like objects inherit from this type.
? Up to you if you want to change it right now though.
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.
I was the one adding the apparently awkward wording in #22052 so might as well update it. (Edit: Done.)
base/array.jl
Outdated
const Vector{T} = Array{T,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.
A line break would be nice here as well.
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.
Done.
1df045f
to
fabf9e5
Compare
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.
Great work here as always!
AppVeyor is #22681. Timeout on Travis i686 (though not the usual one, but should not be related to this PR). |
Thanks. |
""" | ||
Matrix{T} | ||
|
||
Matrix type containing elements of type `T`. Alias for [`Array{T,2}`](@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.
These descriptions of Vector
, Matrix
, AbstractVector
, and AbstractMatrix
seem a bit tautological ("[a Vector
is a] Vector type containing elements of type T
."). Perhaps something the lines of (for Vector
) "[a Vector
is a] one-dimensional dense array with elements of type T
, often used to represent a mathematical vector." would be slightly more informative?
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.
Yea that's better.
No description provided.