Skip to content

Commit 646e21c

Browse files
committed
Sort behaves like R's 'sort(..., na.last=TRUE)'
1 parent b97348e commit 646e21c

File tree

6 files changed

+103
-121
lines changed

6 files changed

+103
-121
lines changed

inst/include/Rcpp/sugar/functions/table.h

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,6 @@
2222
#ifndef Rcpp__sugar__table_h
2323
#define Rcpp__sugar__table_h
2424

25-
#include <Rcpp/sugar/tools/mapcompare.h>
26-
2725
namespace Rcpp{
2826
namespace sugar{
2927

@@ -80,7 +78,7 @@ class Table {
8078
}
8179

8280
private:
83-
typedef RCPP_UNORDERED_MAP<STORAGE, int, MapCompare<STORAGE> >HASH ;
81+
typedef RCPP_UNORDERED_MAP<STORAGE, int, ::Rcpp::traits::comparator_type<STORAGE> >HASH ;
8482
typedef CountInserter<HASH,STORAGE> Inserter ;
8583
HASH hash ;
8684
};
@@ -115,7 +113,7 @@ class Table {
115113
typedef CountInserter<HASH,STORAGE> Inserter ;
116114
HASH hash ;
117115

118-
typedef std::map<STORAGE, int, MapCompare<STORAGE> > SORTED_MAP ;
116+
typedef std::map<STORAGE, int, ::Rcpp::traits::comparator_type<STORAGE> > SORTED_MAP ;
119117
SORTED_MAP map ;
120118

121119
};

inst/include/Rcpp/sugar/tools/mapcompare.h

Lines changed: 0 additions & 100 deletions
This file was deleted.

inst/include/Rcpp/traits/comparator_type.h

Lines changed: 66 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
//
44
// comparator_type.h: Rcpp R/C++ interface class library -- comparator
55
//
6-
// Copyright (C) 2012 Dirk Eddelbuettel and Romain Francois
6+
// Copyright (C) 2012-2014 Dirk Eddelbuettel, Romain Francois and Kevin Ushey
77
//
88
// This file is part of Rcpp.
99
//
@@ -26,20 +26,72 @@
2626
namespace Rcpp{
2727
namespace traits{
2828

29-
class StringCompare {
30-
public:
31-
inline bool operator()( SEXP x, SEXP y) const {
32-
return strcmp( char_nocheck(x), char_nocheck(y) ) < 0 ;
29+
inline bool Rcpp_IsNA(double x) {
30+
return memcmp(
31+
&x,
32+
&NA_REAL,
33+
sizeof(double)
34+
) == 0;
35+
}
36+
37+
inline bool Rcpp_IsNaN(double x) {
38+
return memcmp(
39+
&x,
40+
&R_NaN,
41+
sizeof(double)
42+
) == 0;
43+
}
44+
45+
inline int StrCmp(SEXP x, SEXP y) {
46+
if (x == NA_STRING) return (y == NA_STRING ? 0 : 1);
47+
if (y == NA_STRING) return -1;
48+
if (x == y) return 0; // same string in cache
49+
return strcmp(CHAR(x), CHAR(y));
50+
}
51+
52+
template <typename T>
53+
struct comparator_type {
54+
inline bool operator()(T left, T right) const {
55+
return left < right;
56+
}
57+
};
58+
59+
template <>
60+
struct comparator_type<int> {
61+
inline bool operator()(int left, int right) const {
62+
if (left == NA_INTEGER) return false;
63+
if (right == NA_INTEGER) return true;
64+
return left < right;
65+
}
66+
};
67+
68+
template <>
69+
struct comparator_type<double> {
70+
inline bool operator()(double left, double right) const {
71+
72+
bool leftNaN = (left != left);
73+
bool rightNaN = (right != right);
74+
75+
// this branch inspired by data.table: see
76+
// https://github.com/arunsrinivasan/datatable/commit/1a3e476d3f746e18261662f484d2afa84ac7a146#commitcomment-4885242
77+
if (Rcpp_IsNaN(right) and Rcpp_IsNA(left)) return true;
78+
79+
if (leftNaN != rightNaN) {
80+
return leftNaN < rightNaN;
81+
} else {
82+
return left < right;
3383
}
34-
} ;
35-
36-
template <int RTYPE> struct comparator_type {
37-
typedef std::less< typename storage_type<RTYPE>::type > type ;
38-
} ;
39-
template <> struct comparator_type<STRSXP>{
40-
typedef StringCompare type ;
41-
} ;
42-
84+
85+
}
86+
87+
};
88+
89+
template <>
90+
struct comparator_type<SEXP> {
91+
inline bool operator()(SEXP left, SEXP right) const {
92+
return StrCmp(left, right) < 0;
93+
}
94+
};
4395

4496
}
4597
}

inst/include/Rcpp/vector/Vector.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -245,9 +245,10 @@ class Vector :
245245
Vector& sort(){
246246
typename traits::storage_type<RTYPE>::type* start = internal::r_vector_start<RTYPE>( Storage::get__() ) ;
247247
std::sort(
248-
start, start + size(),
249-
typename traits::comparator_type<RTYPE>::type()
250-
) ;
248+
start,
249+
start + size(),
250+
typename traits::comparator_type<typename traits::storage_type<RTYPE>::type >()
251+
) ;
251252
return *this ;
252253
}
253254

inst/unitTests/cpp/Vector.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -737,3 +737,23 @@ CharacterVector CharacterVector_test_const_proxy(const CharacterVector x){
737737
}
738738
return out ;
739739
}
740+
741+
// [[Rcpp::export]]
742+
NumericVector sort_numeric(NumericVector x) {
743+
return x.sort();
744+
}
745+
746+
// [[Rcpp::export]]
747+
IntegerVector sort_integer(IntegerVector x) {
748+
return x.sort();
749+
}
750+
751+
// [[Rcpp::export]]
752+
CharacterVector sort_character(CharacterVector x) {
753+
return x.sort();
754+
}
755+
756+
// [[Rcpp::export]]
757+
LogicalVector sort_logical(LogicalVector x) {
758+
return x.sort();
759+
}

inst/unitTests/runit.Vector.R

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -658,5 +658,16 @@ if (.runThisTest) {
658658
res <- CharacterVector_test_const_proxy( letters )
659659
checkEquals( res, letters )
660660
}
661+
662+
test.sort <- function() {
663+
num <- setNames( c(1, -1, 4, NA, 5, NaN), letters[1:5] )
664+
checkIdentical( sort_numeric(num), sort(num, na.last=TRUE) )
665+
int <- as.integer(num)
666+
checkIdentical( sort_integer(int), sort(int, na.last=TRUE) )
667+
char <- setNames( sample(letters, 5), LETTERS[1:5] )
668+
checkIdentical( sort_character(char), sort(char, na.last=TRUE) )
669+
lgcl <- as.logical(int)
670+
checkIdentical( sort_logical(lgcl), sort(lgcl, na.last=TRUE) )
671+
}
661672
}
662673

0 commit comments

Comments
 (0)