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

add docstring for SMALL_ALGORITHM #53807

Merged
merged 5 commits into from
Mar 23, 2024
Merged

add docstring for SMALL_ALGORITHM #53807

merged 5 commits into from
Mar 23, 2024

Conversation

danik292
Copy link
Contributor

Part of #52725.

@stevengj

@LilithHafner
Copy link
Member

As I mentioned here, we may want to change this alias from InsertionSort to some other algorithm in the future. By adding a docstring that defines SMALL_ALGORITHM as an alias for InsertionSort, you make it a breaking change to later switch to a new default algorithm for small inputs. Anything we write in this docstring must be true now and stay true in all future versions of Julia unless we explicitly mention that it is unstable.

It would be better to say what this constant is for (i.e. it's an alg good for sorting small vectors) and, optionally, say what it is currently defined as, noting that that is subject to change. See ?Base.Sort.DEFAULT_STABLE for an example

help?> Base.Sort.DEFAULT_STABLE
  DEFAULT_STABLE


  The default sorting algorithm.

  This algorithm is guaranteed to be stable (i.e. it will not reorder elements that
  compare equal). It makes an effort to be fast for most inputs.

  The algorithms used by DEFAULT_STABLE are an implementation detail. See extended
  help for the current dispatch system.

  ───────────────────────────────────────────────────────────────────────────────────

Extended help is available with `??Base.Sort.DEFAULT_STABLE`

@danik292
Copy link
Contributor Author

danik292 commented Mar 21, 2024

So I can say it is am default algorithm for short arreys

@LilithHafner
Copy link
Member

yes

@danik292
Copy link
Contributor Author

Better now

@danik292
Copy link
Contributor Author

Will this PR be marged as one commit to master?

@LilithHafner
Copy link
Member

Will this PR be marged as one commit to master?

Yes.

base/sort.jl Outdated Show resolved Hide resolved
danik292 and others added 2 commits March 21, 2024 21:06
Co-authored-by: Lilith Orion Hafner <lilithhafner@gmail.com>
@LilithHafner
Copy link
Member

Thanks! This should be ready to merge once CI passes

@LilithHafner LilithHafner added docs This change adds or pertains to documentation merge me PR is reviewed. Merge when all tests are passing sorting Put things in order labels Mar 21, 2024
@danik292
Copy link
Contributor Author

You are welcome

base/sort.jl Outdated Show resolved Hide resolved
@stevengj
Copy link
Member

Seems like it could add a bit more explanation, per my suggestion above.

@stevengj stevengj removed the merge me PR is reviewed. Merge when all tests are passing label Mar 22, 2024
@danik292
Copy link
Contributor Author

I will fix it ASAP When I get to the computer

Co-authored-by: Steven G. Johnson <stevenj@mit.edu>
@stevengj stevengj added the merge me PR is reviewed. Merge when all tests are passing label Mar 22, 2024
@LilithHafner LilithHafner merged commit 243f67d into JuliaLang:master Mar 23, 2024
5 of 8 checks passed
@LilithHafner LilithHafner removed the merge me PR is reviewed. Merge when all tests are passing label Mar 23, 2024
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 sorting Put things in order
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants