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

Rename buffer to scratch in sorting #47172

Merged
merged 2 commits into from
Nov 6, 2022
Merged

Rename buffer to scratch in sorting #47172

merged 2 commits into from
Nov 6, 2022

Conversation

LilithHafner
Copy link
Member

sort!(x; buffer) => sort!(x; scratch)

It's not too late to rename this again if folks want to. This has never been public, documented, or part of a release or release candidate.

This PR is very similar to #45711

From @StefanKarpinski's comment here: #45222 (comment)

cc those who have previously expressed interest in the name of this thing: @oscardssmith @musm @StefanKarpinski @pcjentsch @longemen3000 @mkitti @jw3126 @halleysfifthinc @nalimilan

@LilithHafner LilithHafner added the domain:sorting Put things in order label Oct 15, 2022
@halleysfifthinc
Copy link
Contributor

Rereading the discussion in #45330 , buffer appears to be the most popular name option. One point against scratch is that (to me) the purpose isn't immediately obvious from the name. To be fair, this is probably something that shouldn't/wouldn't be used without first reading the docstring, but buffer (or even scratchspace) are more familiar and/or self-explanatory.

I also just noticed that this isn't documented in the sort docstring. Was that intentional?

base/sort.jl Show resolved Hide resolved
@StefanKarpinski
Copy link
Sponsor Member

I quite like this name. Much clearer imo than "buffer"; the main issue I have with "buffer" is that it is often used for input or output vectors. This is neither—it is only used as a temporary scratch space during the sorting operation and I don't think there's a clearer or more concise name that conveys that than scratch.

@jw3126
Copy link
Contributor

jw3126 commented Oct 18, 2022

(or even scratchspace) are more familiar and/or self-explanatory.

+1 for scratchspace. It is even more self-explanatory and since this is a low level optimization typing a few more characters seems fine to me.

@StefanKarpinski
Copy link
Sponsor Member

StefanKarpinski commented Oct 18, 2022

I considered that but the name is used internally and having it be longer than necessary is therefore annoying. I'm not sure scratchspace is clearer than scratch since what else is scratch the start of? But sure, scratchspace would also be good.

@bramtayl
Copy link
Contributor

what else is scratch the start of?

A curated list courtesy autocomplete: the scratchard equation, scratcharia, scratchnapped, scratchmen apoo, scratch-o, scratchtasia

@LilithHafner
Copy link
Member Author

I also just noticed that this isn't documented in the sort docstring. Was that intentional?

Yes. It's not part of the public API because it is new so we might want to change it and nobody has requested that it become a part of the public API. Unlike most features, this one does not suffer much from being internal because its main use case is high-level sorting functions such as sort!(::AbstractMatrix).

@KristofferC KristofferC added the status:merge me PR is reviewed. Merge when all tests are passing label Nov 4, 2022
@DilumAluthge DilumAluthge merged commit a768d0c into master Nov 6, 2022
@DilumAluthge DilumAluthge deleted the buffer-scratch branch November 6, 2022 00:11
@DilumAluthge DilumAluthge removed the status:merge me PR is reviewed. Merge when all tests are passing label Nov 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:sorting Put things in order
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants