Update performance-tips.md #22479
Update performance-tips.md #22479
Conversation
New paragraph about the use of `convert` and type stability.
clarified my clarification
kshyatt
added
domain:docs
doc/src/manual/performance-tips.md
Outdated
| @@ -477,6 +477,12 @@ Here, we happened to know that the first element of `a` would be an [`Int32`](@r | |||
| an annotation like this has the added benefit that it will raise a run-time error if the | |||
| value is not of the expected type, potentially catching certain bugs earlier. | |||
|
|
|||
| A slightly more generic way to annotate the type of `x` is `x = convert(Int32,a[1])::Int32`. The | |||
There was a problem hiding this comment.
Perhaps rephrase this to "A slightly more generic way to annotate the type of x is shown by the example of...". It took me a second to realize you are referring to the ::Int32 bit and the fact that convert is involved is somewhat incidental (if I've understood correctly).
There was a problem hiding this comment.
This paragraph is supposed to make two points: First, to increase genericity, a convert function can be used rather than a direct annotation. In other words, the use of convert allows more possible types for a[1]. The second point (which is the point that surprised me) is that if convert is used in this manner, it needs a type annotation else it is type-unstable.
There was a problem hiding this comment.
The second part comes through pretty well but your response to me here, if worked into what you already have, would make it a lot clearer I think. Thanks for adding this!
Rewrote new text following suggestion of @kshyatt
doc/src/manual/performance-tips.md
Outdated
| @@ -477,6 +477,14 @@ Here, we happened to know that the first element of `a` would be an [`Int32`](@r | |||
| an annotation like this has the added benefit that it will raise a run-time error if the | |||
| value is not of the expected type, potentially catching certain bugs earlier. | |||
|
|
|||
| In the case that the type of `a[1]` is not known precisely, `x` can be declared via | |||
| `x = convert(Int32,a[1])::Int32`. The | |||
| use of the `convert` function allows `a[1]` to be any object convertible to an `Int32` (such as `UInt8`), thus | |||
There was a problem hiding this comment.
Perhaps worth adding a reference to convert? E.g. [`convert`](@ref)
doc/src/manual/performance-tips.md
Outdated
| increasing the genericity of the code by loosening the type requirement. | ||
| Notice that the `convert` function itself needs a type annotation in this context in order to achieve type stability. | ||
| This is because the compiler cannot deduce the type of the return value of a function, even | ||
| `convert`, unless the types of all its arguments are known. |
There was a problem hiding this comment.
Perhaps "...the types of all the arguments...", since "its" could refer to the compiler in this sentence?
doc/src/manual/performance-tips.md
Outdated
| @@ -477,6 +477,14 @@ Here, we happened to know that the first element of `a` would be an [`Int32`](@r | |||
| an annotation like this has the added benefit that it will raise a run-time error if the | |||
| value is not of the expected type, potentially catching certain bugs earlier. | |||
|
|
|||
| In the case that the type of `a[1]` is not known precisely, `x` can be declared via | |||
| `x = convert(Int32,a[1])::Int32`. The | |||
There was a problem hiding this comment.
Double space before "The" ?
doc/src/manual/performance-tips.md
Outdated
| `x = convert(Int32,a[1])::Int32`. The | ||
| use of the `convert` function allows `a[1]` to be any object convertible to an `Int32` (such as `UInt8`), thus | ||
| increasing the genericity of the code by loosening the type requirement. | ||
| Notice that the `convert` function itself needs a type annotation in this context in order to achieve type stability. |
There was a problem hiding this comment.
Perhaps "Notice that convert itself..." ?
More small edits as per comments from @nalimilan and @fredrikekre
doc/src/manual/performance-tips.md
Outdated
| In the case that the type of `a[1]` is not known precisely, `x` can be declared via | ||
| `x = convert(Int32,a[1])::Int32`. The use of the [`convert`](@ref) function allows `a[1]` | ||
| to be any object convertible to an `Int32` (such as `UInt8`), thus increasing the genericity | ||
| of the code by loosening the type requirement. Notice that `convert` itself needs a type |
There was a problem hiding this comment.
Trailing space which causes CI to fail, same on the next 2 lines.
Removed trailing spaces
* Update performance-tips.md New paragraph about the use of `convert` and type stability. * Update performance-tips.md clarified my clarification * Update performance-tips.md Rewrote new text following suggestion of @kshyatt * Update performance-tips.md More small edits as per comments from @nalimilan and @fredrikekre * Update performance-tips.md Removed trailing spaces
|
@tkelman Would have been nice to remove mentions of our usernames from the commit message. |
I added a paragraph concerning something that surprised me about
convert.