Skip to content

Commit

Permalink
Revert "Fix missing value propagation in as_integers/doubles() (#265)"
Browse files Browse the repository at this point in the history
This reverts commit 3c87798.
  • Loading branch information
romainfrancois committed May 24, 2023
1 parent 3c87798 commit 2df5613
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 60 deletions.
3 changes: 0 additions & 3 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
# cpp11 (development version)

* `as_doubles()` and `as_integers()` now propagate missing values correctly
(#265).

* Fixed a performance issue related to nested `unwind_protect()` calls (#298).

* Minor performance improvements to the cpp11 protect code. (@kevinushey)
Expand Down
5 changes: 0 additions & 5 deletions cpp11test/src/test-doubles.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -393,11 +393,6 @@ context("doubles-C++") {
e.push_back("a");
e.push_back("b");
expect_error(cpp11::as_doubles(e));

cpp11::writable::integers na;
na.push_back(cpp11::na<int>());
cpp11::doubles na2(cpp11::as_doubles(na));
expect_true(cpp11::is_na(na2[0]));
}

test_that("doubles operator[] and at") {
Expand Down
9 changes: 1 addition & 8 deletions cpp11test/src/test-integers.cpp
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
#include "Rversion.h"

#include "cpp11/doubles.hpp"
#include "cpp11/function.hpp"
#include "cpp11/integers.hpp"

#include "cpp11/doubles.hpp"
#include "cpp11/strings.hpp"

#include <testthat.h>
Expand Down Expand Up @@ -38,11 +36,6 @@ context("integers-C++") {
expect_true(t[2] == 100000);
expect_true(t[3] == 100000);
expect_true(TYPEOF(t) == INTSXP);

cpp11::writable::doubles na;
na.push_back(cpp11::na<double>());
cpp11::integers na2(cpp11::as_integers(na));
expect_true(cpp11::is_na(na2[0]));
}

test_that("integers.push_back()") {
Expand Down
4 changes: 2 additions & 2 deletions inst/include/cpp11/as.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ using enable_if_c_string = enable_if_t<std::is_same<T, const char*>::value, R>;

// https://stackoverflow.com/a/1521682/2055486
//
inline bool is_convertible_without_loss_to_integer(double value) {
inline bool is_convertable_without_loss_to_integer(double value) {
double int_part;
return std::modf(value, &int_part) == 0.0;
}
Expand All @@ -100,7 +100,7 @@ enable_if_integral<T, T> as_cpp(SEXP from) {
return NA_INTEGER;
}
double value = REAL_ELT(from, 0);
if (is_convertible_without_loss_to_integer(value)) {
if (is_convertable_without_loss_to_integer(value)) {
return value;
}
}
Expand Down
43 changes: 16 additions & 27 deletions inst/include/cpp11/doubles.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -135,44 +135,33 @@ typedef r_vector<double> doubles;

} // namespace writable

template <>
inline double na() {
return NA_REAL;
}

template <>
inline bool is_na(const double& x) {
return ISNA(x);
}

// forward declarations
typedef r_vector<int> integers;

template <>
int na();

template <>
int r_vector<int>::operator[](const R_xlen_t pos) const;

inline doubles as_doubles(sexp x) {
if (TYPEOF(x) == REALSXP) {
return as_cpp<doubles>(x);
} else if (TYPEOF(x) == INTSXP) {
}

else if (TYPEOF(x) == INTSXP) {
integers xn = as_cpp<integers>(x);
R_xlen_t len = xn.size();
writable::doubles ret(len);
for (R_xlen_t i = 0; i < len; ++i) {
int el = xn[i];
if (is_na(el)) {
ret[i] = na<double>();
} else {
ret[i] = static_cast<double>(el);
}
size_t len = xn.size();
writable::doubles ret;
for (size_t i = 0; i < len; ++i) {
ret.push_back(static_cast<double>(xn[i]));
}
return ret;
}

throw type_error(REALSXP, TYPEOF(x));
}

template <>
inline double na() {
return NA_REAL;
}

template <>
inline bool is_na(const double& x) {
return ISNA(x);
}
} // namespace cpp11
23 changes: 8 additions & 15 deletions inst/include/cpp11/integers.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -145,32 +145,25 @@ inline int na() {
return NA_INTEGER;
}

// forward declarations
typedef r_vector<double> doubles;

template <>
bool is_na(const double& x);
// forward declaration

template <>
double r_vector<double>::operator[](const R_xlen_t pos) const;
typedef r_vector<double> doubles;

inline integers as_integers(sexp x) {
if (TYPEOF(x) == INTSXP) {
return as_cpp<integers>(x);
} else if (TYPEOF(x) == REALSXP) {
doubles xn = as_cpp<doubles>(x);
R_xlen_t len = xn.size();
writable::integers ret(len);
for (R_xlen_t i = 0; i < len; ++i) {
size_t len = (xn.size());
writable::integers ret = writable::integers(len);
for (size_t i = 0; i < len; ++i) {
double el = xn[i];
if (is_na(el)) {
ret[i] = na<int>();
} else if (is_convertible_without_loss_to_integer(el)) {
ret[i] = static_cast<int>(el);
} else {
if (!is_convertable_without_loss_to_integer(el)) {
throw std::runtime_error("All elements must be integer-like");
}
ret[i] = (static_cast<int>(el));
}

return ret;
}

Expand Down

0 comments on commit 2df5613

Please sign in to comment.