sorting API refactor using keywords #3665

Merged
merged 7 commits into from Jul 11, 2013

Projects

None yet

4 participants

@StefanKarpinski
Member

Quoting the main commit:

The idea here is that the ordering and algorithm for sorting are two orthogonal parameters that have sensible defaults: neither, both or either can be independently specified. The resulting API is slightly more verbose but also somewhat more self-documenting.

Another sign that this might be on the right track is that this change reduces the LOC of base/sort.jl by 41 lines and reduces the method count of sort, for example, from 12 to 6.

Still needs deprecation methods before merging, but I thought I'd put it out there for feedback.

Note: I really wish that the lt and by ordering variations could be cleanly specified this way too – that would really cut the API down to a nice lean size without sacrificing any functionality. But I can't figure out a good way to do it.

@StefanKarpinski
Member

Ok, I took a crack at using keywords for by and lt as well and I really rather like the resulting API. It eliminates the need for sortby! and sortby – and generalizes naturally to other order-related functions without blowing up the export set or requiring the use of awkward types that really should be implementation details of the sorting code.

It has, however, revealed what seems to be a method dispatch bug. Will file an issue.

StefanKarpinski added some commits Jul 9, 2013
@StefanKarpinski StefanKarpinski sorting: make `order` and `alg` keyword arguments.
The idea here is that the ordering and algorithm for sorting are
two orthogonal parameters that have sensible defaults: neither,
both or either can be independently specified. The resulting API
is slightly more verbose but also somewhat more self-documenting.

Another sign that this might be on the right track is that this
change reduces the LOC of base/sort.jl by 41 lines and reduces the
method count of sort, for example, from 12 to 6.
73ecfd8
@StefanKarpinski StefanKarpinski sorting: move matrix sorts up with other generic sorting functions. 738c230
@StefanKarpinski StefanKarpinski sorting: eliminate a few nans2{left,right} definitions via defaults. e5498d1
@StefanKarpinski StefanKarpinski sorting: keyword arguments for `by` and `lt` as well. 2409b35
@StefanKarpinski StefanKarpinski sorting: make ReverseOrder parametric -- can reverse any ordering. 3326e0c
@StefanKarpinski StefanKarpinski sorting: add `rev` boolean keyword to reverse any sort ordering.
Also added lt, by, order, rev keywords to the searchsorted* funcs.
7bab350
@StefanKarpinski StefanKarpinski sorting: deprecations for the old sorting API af97a6c
@StefanKarpinski StefanKarpinski merged commit f951d7c into master Jul 11, 2013
@StefanKarpinski StefanKarpinski deleted the sk/sort-keywords branch Jul 11, 2013
@StefanKarpinski StefanKarpinski referenced this pull request Jul 19, 2013
Closed

0.2 release notes #2581

@stevengj
Member

@StefanKarpinski gets the hornéd hat of shame for checking this in without updating the documentation...

@ViralBShah
Member

I hate to do this.

+1

@StefanKarpinski
Member

I'm always wearing a horned hat of shame.

@GunnarFarneback

This ought to give a deprecation message, I suppose:

julia> sort(>, [1:5])
ERROR: no method sort(Function,Array{Int64,1})
@StefanKarpinski
Member

Yes, thanks – I missed that one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment