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.

Sign up for GitHub

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

Jump to bottom

Fix Issue 8341 - topN(zip()) doesn't work #6156

Merged
merged 1 commit into from Mar 12, 2018
Merged

Fix Issue 8341 - topN(zip()) doesn't work #6156

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 4 additions & 5 deletions std/algorithm/mutation.d
View file
Open in desktop
Expand Up @@ -84,11 +84,6 @@ import std.typecons; // : tuple, Tuple;

// bringToFront
/**
The `bringToFront` function has considerable flexibility and
usefulness. It can rotate elements in one buffer left or right, swap
buffers of equal length, and even move elements across disjoint
buffers of different types and different lengths.

`bringToFront` takes two ranges `front` and `back`, which may
be of different types. Considering the concatenation of `front` and
`back` one unified range, `bringToFront` rotates that unified
Expand All @@ -104,6 +99,10 @@ in ranges, not as a string function.
Performs $(BIGOH max(front.length, back.length)) evaluations of $(D
swap).

The `bringToFront` function can rotate elements in one buffer left or right, swap
buffers of equal length, and even move elements across disjoint
buffers of different types and different lengths.

Preconditions:

Either `front` and `back` are disjoint, or `back` is
Expand Down
33 changes: 23 additions & 10 deletions std/algorithm/sorting.d
View file
Open in desktop
Expand Up @@ -112,8 +112,9 @@ Params:
sorted.
*/
void completeSort(alias less = "a < b", SwapStrategy ss = SwapStrategy.unstable,
RandomAccessRange1, RandomAccessRange2)(SortedRange!(RandomAccessRange1, less) lhs, RandomAccessRange2 rhs)
if (hasLength!(RandomAccessRange2) && hasSlicing!(RandomAccessRange2))
Lhs , Rhs)(SortedRange!(Lhs, less) lhs, Rhs rhs)
if (hasLength!(Rhs) && hasSlicing!(Rhs)
&& hasSwappableElements!Lhs && hasSwappableElements!Rhs)
{
import std.algorithm.mutation : bringToFront;
import std.range : chain, assumeSorted;
Expand Down Expand Up @@ -393,7 +394,8 @@ the relative ordering of all elements `a`, `b` in the left part of `r`
for which `predicate(a) == predicate(b)`.
*/
Range partition(alias predicate, SwapStrategy ss, Range)(Range r)
if (ss == SwapStrategy.stable && isRandomAccessRange!(Range) && hasLength!Range && hasSlicing!Range)
if (ss == SwapStrategy.stable && isRandomAccessRange!(Range) && hasLength!Range &&
hasSlicing!Range && hasSwappableElements!Range)
{
import std.algorithm.mutation : bringToFront;

Expand Down Expand Up @@ -611,7 +613,7 @@ Keynote), Andrei Alexandrescu.
*/
size_t pivotPartition(alias less = "a < b", Range)
(Range r, size_t pivot)
if (isRandomAccessRange!Range && hasLength!Range && hasSlicing!Range)
if (isRandomAccessRange!Range && hasLength!Range && hasSlicing!Range && hasAssignableElements!Range)
{
assert(pivot < r.length || r.length == 0 && pivot == 0);
if (r.length <= 1) return 0;
Expand All @@ -631,7 +633,7 @@ if (isRandomAccessRange!Range && hasLength!Range && hasSlicing!Range)
auto p = r[0];
// Plant the pivot in the end as well as a sentinel
size_t lo = 0, hi = r.length - 1;
auto save = move(r[hi]);
auto save = r.moveAt(hi);
r[hi] = p; // Vacancy is in r[$ - 1] now
// Start process
for (;;)
Expand Down Expand Up @@ -956,7 +958,7 @@ makeIndex(
RangeIndex)
(Range r, RangeIndex index)
if (isForwardRange!(Range) && isRandomAccessRange!(RangeIndex)
&& is(ElementType!(RangeIndex) : ElementType!(Range)*))
&& is(ElementType!(RangeIndex) : ElementType!(Range)*) && hasAssignableElements!RangeIndex)
{
import std.algorithm.internal : addressOf;
import std.exception : enforce;
Expand All @@ -980,7 +982,7 @@ void makeIndex(
(Range r, RangeIndex index)
if (isRandomAccessRange!Range && !isInfinite!Range &&
isRandomAccessRange!RangeIndex && !isInfinite!RangeIndex &&
isIntegral!(ElementType!RangeIndex))
isIntegral!(ElementType!RangeIndex) && hasAssignableElements!RangeIndex)
{
import std.conv : to;
import std.exception : enforce;
Expand Down Expand Up @@ -1450,7 +1452,7 @@ Returns:
template multiSort(less...) //if (less.length > 1)
{
auto multiSort(Range)(Range r)
if (validPredicates!(ElementType!Range, less))
if (validPredicates!(ElementType!Range, less) && hasSwappableElements!Range)
{
import std.meta : AliasSeq;
import std.range : assumeSorted;
Expand Down Expand Up @@ -2900,7 +2902,7 @@ SortedRange!(R, ((a, b) => binaryFun!less(unaryFun!transform(a),
unaryFun!transform(b))))
schwartzSort(alias transform, alias less = "a < b",
SwapStrategy ss = SwapStrategy.unstable, R)(R r)
if (isRandomAccessRange!R && hasLength!R)
if (isRandomAccessRange!R && hasLength!R && hasSwappableElements!R)
{
import std.conv : emplace;
import std.range : zip, SortedRange;
Expand Down Expand Up @@ -3100,7 +3102,8 @@ Stable topN has not been implemented yet.
auto topN(alias less = "a < b",
SwapStrategy ss = SwapStrategy.unstable,
Range)(Range r, size_t nth)
if (isRandomAccessRange!(Range) && hasLength!Range && hasSlicing!Range)
if (isRandomAccessRange!(Range) && hasLength!Range &&
hasSlicing!Range && hasAssignableElements!Range)
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be hasSwappableElements?

Copy link
Member Author

Choose a reason for hiding this comment

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

No it calls pivotPartition which calls moveAt and assigns to the element:

auto save = r.moveAt(hi);
...
r[lo] = save;

hasSwappableElements just requires that swap(a, b) works.

{
static assert(ss == SwapStrategy.unstable,
"Stable topN not yet implemented");
Expand Down Expand Up @@ -3133,6 +3136,16 @@ if (isRandomAccessRange!(Range) && hasLength!Range && hasSlicing!Range)
assert(v[n] == 9);
}

// https://issues.dlang.org/show_bug.cgi?id=8341
unittest
{
import std.algorithm.comparison : equal;
import std.typecons : tuple;
auto a = [10, 30, 20];
auto b = ["c", "b", "a"];
assert(topN!"a[0] > b[0]"(zip(a, b), 2).equal([tuple(20, "a"), tuple(30, "b")]));
}

private @trusted
void topNImpl(alias less, R)(R r, size_t n, ref bool useSampling)
{
Expand Down