add docstring for small threshold #54602
Conversation
base/sort.jl
Outdated
| """ | ||
| SMALL_THRESHOLD | ||
|
|
||
| SMALL_THRESHOLD is constant for internally decide, whatever is arrey considered small or big |
There was a problem hiding this comment.
A few pointers:
This has spelling errors and is not grammatically correct (in fact I have trouble figuring out what it tries to say).
In addition: this line shouldn't be indented, there should be an empty line before this docstring, and it shouldn't end in an empty line.
There was a problem hiding this comment.
Ok I will fix it when I get to the computer
mbauman
added
docs
| """ | ||
| SMALL_THRESHOLD | ||
|
|
||
| SMALL_THRESHOLD is a constant that is used internally to decide what is considered to be small or large. |
There was a problem hiding this comment.
That's a lot better already, but it isn't yet all that helpful as it gives no indication about the context in which it is used, i.e. what "small" or "large" mean or affect.
I think something to the effect of: for vectors with less (?) than SMALL_THRESHOLD elements, sorting switches to a different algorithm SMALL_ALGORITHM, otherwise it uses the big algorithm. This is in particular relevant for recursive sorting algorithms which reduce to smaller sublists, sort those and combine the result.
Note that SMALL_ALGORITHM also has a docstring.
There was a problem hiding this comment.
The threshold at which to switch sorting algorithms based on size is also quite variable. SMALL_THRESHOLD is only used in some cases (e.g. QuickSort and MergeSort)
|
Why is this a public/exported constant at all? Shouldn't it just be private? |
|
It should be private but it's exported from the public module Sort. julia> using Base.Sort
julia> SMALL_THRESHOLD
20
julia> VERSION
v"1.0.5" |
Co-authored-by: Max Horn <max@quendi.de>
|
What is a right use of constant witch i should describe in docstring? |
|
For the most part, I don't think folks should be using it in code. They may wish to refer to it when understanding how the internals work, though. |
Part of #52725