rename: sort_by => sortby ?

[StefanKarpinski]

See JuliaData/DataFrames.jl#139.

[kmsquire]

Just to add: most of the sort-specific functions have _by and _r forms. The _r forms are not consistent with sortr. I used underscores in everything to make the names easier to read (I felt), but didn't touch sortr to avoid messing too much with code.

I'm not at all tied to using underscores, but I do think they should be made consistent.

[johnmyleswhite]

I'm also not really for or against underscores, but am very sold on consistency.

[ViralBShah]

Agree. Consistency is the most important to have.

[JeffBezanson]

I prefer no-underscores, since if you try to use them everywhere consistently there are simply too many. We don't want cum_sum, is_equal, size_of etc.

[johnmyleswhite]

I'm coming around to the no-underscores rule.

[ViralBShah]

Irrespective of the underscores, we should certainly not have sortr and sort_r doing different things. Ok to remove the underscores in the various sort functions.

@kmsquire How do sortr and sort_r differ?

[kmsquire]

Sorry, to be more clear, there is no sort_r, there are only specialized sort versions which have _r appended, instead of just r, for reverse sorting. Specifically, the following functions are defined:

• sort, sortr, sort_by, sortperm, sortperm_r, sortperm_by
• quicksort, quicksort_r, quicksort_by (*)
• mergesort, mergesort_r, mergesortby, mergesortperm, mergesortpermr, mergesortpermby
• insertionsort, insertionsort_r, insertionsort_by, insertionsort_perm, insertionsort_perm_r, insertionsort_perm_by
• timsort, timsort_r, timsortby, timsort_perm, timsort_perm_r, timsort_perm_by
• search_sorted, search_sorted_r, search_sorted_by
• search_sorted_first, search_sorted_first_r, search_sorted_first_by
• search_sorted_last, search_sorted_last_r, search_sorted_last_by
• issorted, issorted_r, issorted_by
• select, select_r, select_by

(*) there is no quicksort_perm

Without underscores, these we would have

• sort, sortr, sortby, sortperm, sortpermr, sortpermby
• quicksort, quicksortr, quicksortby
• mergesort, mergesortr, mergesortby, mergesortperm, mergesortpermr, mergesortpermby
• insertionsort, insertionsortr, insertionsortby, insertionsortperm, insertionsortpermr, insertionsortpermby
• timsort, timsortr, timsortby, timsortperm, timsortpermr, timsortpermby
• searchsorted, searchsortedr, searchsortedby
• searchsortedfirst, searchsortedfirstr, searchsortedfirstby
• searchsortedlast, searchsortedlastr, searchsortedlastby
• issorted, issortedr, issortedby
• select, selectr, selectby

Seeing this mess, I'm wondering if we could get rid of all of the specialized sorts, and just dispatch by singleton classes--i.e., something like

type QuickSort end
type InsertionSort end
type MergeSort end
type TimSort end

quicksort = QuickSort()
insertionsort = InsertionSort()
mergesort = MergeSort()
timsort = TimSort()

function sort(v::Vector)
   ...
end

function sort(s::Quicksort, v::Vector)
   ...
end

function sort(s::Timsort, v::Vector)
   ...
end

...

It would really clean up the mess of functions above, and getting rid of underscores wouldn't be a big deal.

Kevin

[johnmyleswhite]

Types as tags!

[toivoh]

Writing it as

function sort(::Type{Quicksort}, v::Vector)
   ...
end

allows to call directly using the type instead, sort(Quicksort, v). A little cleaner, I think.

[toivoh]

Also, agree that this seems like a good way to clean up the mess.

[johnmyleswhite]

I also think the sort(::Type{Quicksort}, v::Vector) approach is cleaner.

[kmsquire]

Yes, that does look a bit cleaner--thanks @toivoh. I'll work up a patch.

[diegozea]

Create empty types only for dispatch, and remember passing through functions, looks awkward to me.
Why not optional parameters instead? [ http://docs.julialang.org/en/latest/stdlib/options/ ]

[ViralBShah]

I am not fully convinced of the ::Type{QuickSort} approach. I can't quite make up my mind.

[ViralBShah]

I also have to say that some of the non-underscore names look very weird.

[StefanKarpinski]

The compiler will specialize methods on the ::Type{QuickSort} argument, leading to fast code. Using an optional parameter would lead to run-time overhead everywhere. I'm not sure how I feel about this approach. Might need to sketch it out a little to see how it feels.

[diegozea]

Maybe can be more consistent a unique Type for flags on functions ( something like ::Type{Flag} ) and do something like

const = flag"QuickSort" # Type Flag

sort(v::Vector) = ... # Default
sort(v::Vector,f::Flag) = if f==flag"QuickSort" ...

?

[johnmyleswhite]

I think using a Flag type is probably not more efficient than using symbols.

[diegozea]

Maybe can be good a rule for naming Types with only are using has function options, like a Opt... or Flag... prefix ?

sort(OptQuicksort, v)

The name of the Function in Uppercase can be a good option if you want to use autocompletion of REPL ;) Like

sort(SORTQuicksort, v)
sort(SORT_Quicksort, v)

sort(SortQuicksort, v)
sort(Sort_Quicksort, v)

But it's true that function name can looks redundant and can be too large
Given they are constants, all uppercase letters can be an option too:

sort(QUICKSORT, v)

I don't know... a more simple and easy to tipping idea can be better

[pao]

I'm still an advocate for real sum types to replace the empty type enum pattern, but still provide us with its dispatch and error-finding advantages.

[ViralBShah]

I am actually ok with the various sort functions having different names (although you may wonder why I am not ok with the same in the case of norm) and people having to load the module for that. Only the defaults can be exported.

[StefanKarpinski]

@pao: can you give an example of what you mean by "real sum types" here?

[pao]

It's primarily a syntactic thing that could probably be fixed with a macro and Jeff no longer objecting to the pattern, but ideally if there's any overhead in creating an empty CompositeKind you could get rid of that as well:

Haskell:

data SortAlgorithm = QuickSort | InsertionSort | MergeSort -- etc.

Scala:

sealed abstract class SortAlgorithm
case object QuickSort extends SortAlgorithm
case object InsertionSort extends SortAlgorithm
case object MergeSort extends SortAlgorithm
// etc. (ugly syntax but "case" makes these enum singletons)

[kmsquire]

I was actually thinking of @pao's Scala example as precedence for this pattern, although I was wondering how it would go over.

@ViralBShah, even if we end up keeping all of the different names, do you think we should keep or get rid of the underscores? If we get rid of all of them, the names are a bit unwieldy. (c.f. #1539.)

I have a patch almost finished, and should be able to submit it today for further discussion.

[JeffBezanson]

Superseded by new sort API.