Skip to content

Doc: The default sorting alg. is stable from 1.9#47579

Merged
LilithHafner merged 16 commits intoJuliaLang:masterfrom
petvana:pv/sort-docs
Nov 21, 2022
Merged

Doc: The default sorting alg. is stable from 1.9#47579
LilithHafner merged 16 commits intoJuliaLang:masterfrom
petvana:pv/sort-docs

Conversation

@petvana
Copy link
Copy Markdown
Member

@petvana petvana commented Nov 15, 2022

This PR updates the docs after #45222 and resolves my comments from #47297 (comment)

Notice PartialQuickSort(::Integer) is about to be deprecated in #47297, and thus the documentation is removed as well.

Cc: @LilithHafner

Also relates to #47304

@petvana petvana added docs This change adds or pertains to documentation sorting Put things in order labels Nov 15, 2022
@PallHaraldsson
Copy link
Copy Markdown
Contributor

PallHaraldsson commented Nov 15, 2022

@LilithHafner Thanks for your work on sorting!

O(n²) is worst-case complexity, but it gets vanishingly unlikely

I know that, not worried, still might introsort variant be better; it avoids that? Might it be faster (with or without randomization)?

@petvana
Copy link
Copy Markdown
Member Author

petvana commented Nov 15, 2022

@PallHaraldsson Related discussion is here #45222 (comment)

@petvana
Copy link
Copy Markdown
Member Author

petvana commented Nov 18, 2022

@LilithHafner Can you please check the wording because it would be nice to merge this docs (and backport it) before 1.9 is out.

@petvana petvana added the backport 1.9 Change should be backported to release-1.9 label Nov 18, 2022
LilithHafner
LilithHafner reviewed Nov 18, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@LilithHafner LilithHafner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, thanks for the bump! I had this review lying around but hadn't submitted it.

@petvana
Copy link
Copy Markdown
Member Author

petvana commented Nov 18, 2022 •
edited
Loading

@LilithHafner Thank you for the comments. I've resolved them + I've removed comments about v1.9 (7a66f76) as it is already noted in compat.

Therefore, it seems ready from my side.

LilithHafner
LilithHafner approved these changes Nov 21, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@LilithHafner LilithHafner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed a couple of typos and tweaked some wording. If it still looks good to you, I'm ready to merge.

doc/src/base/sort.md
map(x->issorted(x[k2]), (s, ps, qs)) # => (true, false, true)
s[1:k] == ps[1:k] # => true
s[k2] == qs[k2] # => true
```jldoctest
Copy link
Copy Markdown
Member

@LilithHafner LilithHafner Nov 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making this a doctest!

@petvana
Copy link
Copy Markdown
Member Author

petvana commented Nov 21, 2022

@LilithHafner Thanks for the changes. I've removed *appear* to be stable you reverted because it seems to me to be slightly misleading, since the default algorithm is always stable now (unless some library changes this behavior). Otherwise, it can be merged.

@LilithHafner
Copy link
Copy Markdown
Member

LilithHafner commented Nov 21, 2022

I've removed *appear* to be stable you reverted because it seems to me to be slightly misleading,

Agreed.

"" > "appear to be stable" > "appear to be fast"

@LilithHafner LilithHafner added the merge me PR is reviewed. Merge when all tests are passing label Nov 21, 2022
@LilithHafner LilithHafner merged commit c5fe17b into JuliaLang:master Nov 21, 2022
@LilithHafner LilithHafner removed the merge me PR is reviewed. Merge when all tests are passing label Nov 21, 2022
@LilithHafner
Copy link
Copy Markdown
Member

LilithHafner commented Nov 21, 2022

Thanks!

KristofferC pushed a commit that referenced this pull request Nov 28, 2022
@petvana @LilithHafner @KristofferC
Doc: The default sorting alg. is stable from 1.9 (#47579)
4b4521f 
* Update doc/src/base/sort.md

* Update docs: The default sorting alg. is stable

* Compat 1.9 for QuickSort to be stable

* Specify the default algorithm

* Use example from InlineStrings.jl

* Change example to jldoctest

* Remove "*appear* to be stable." as slightly misleading.

Co-authored-by: Lilith Orion Hafner <lilithhafner@gmail.com>
(cherry picked from commit c5fe17b)
KristofferC pushed a commit that referenced this pull request Nov 28, 2022
@petvana @LilithHafner @KristofferC
Doc: The default sorting alg. is stable from 1.9 (#47579)
b5d6b03 
* Update doc/src/base/sort.md

* Update docs: The default sorting alg. is stable

* Compat 1.9 for QuickSort to be stable

* Specify the default algorithm

* Use example from InlineStrings.jl

* Change example to jldoctest

* Remove "*appear* to be stable." as slightly misleading.

Co-authored-by: Lilith Orion Hafner <lilithhafner@gmail.com>
(cherry picked from commit c5fe17b)
@KristofferC KristofferC mentioned this pull request Nov 28, 2022
Merged
51 tasks
@LilithHafner LilithHafner mentioned this pull request Dec 4, 2022
Closed
@KristofferC KristofferC mentioned this pull request Dec 14, 2022
Merged
26 tasks
@KristofferC KristofferC removed the backport 1.9 Change should be backported to release-1.9 label Dec 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@LilithHafner LilithHafner LilithHafner approved these changes

Assignees

No one assigned

Labels

docs This change adds or pertains to documentation sorting Put things in order

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@petvana @PallHaraldsson @LilithHafner @KristofferC