Skip to content

Commit

Permalink
Speculative fix for vroom issue (#512)
Browse files Browse the repository at this point in the history 
* Speculative fix for vroom issue

* Slightly more correct `cref()`

* Add a NEWS bullet

---------

Co-authored-by: Jenny Bryan <jenny.f.bryan@gmail.com>
  • Loading branch information
@DavisVaughan @jennybc
DavisVaughan and jennybc committed Sep 21, 2023
1 parent 3691c68 commit 2f96e7b
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 35 deletions.
2 changes: 2 additions & 0 deletions NEWS.md
View file
@@ -1,5 +1,7 @@
# vroom (development version)

* Internal change for compatibility with cpp11 >= 0.4.6 (@DavisVaughan).

# vroom 1.6.3

* No user-facing changes.
Expand Down
24 changes: 12 additions & 12 deletions R/cpp11.R
View file
Expand Up @@ -28,6 +28,18 @@ convert_connection <- function(in_con, out_con, from, to) {
.Call(`_vroom_convert_connection`, in_con, out_con, from, to)
}

vroom_ <- function(inputs, delim, quote, trim_ws, escape_double, escape_backslash, comment, skip_empty_rows, skip, n_max, progress, col_names, col_types, col_select, name_repair, id, na, locale, guess_max, num_threads, altrep) {
.Call(`_vroom_vroom_`, inputs, delim, quote, trim_ws, escape_double, escape_backslash, comment, skip_empty_rows, skip, n_max, progress, col_names, col_types, col_select, name_repair, id, na, locale, guess_max, num_threads, altrep)
}

has_trailing_newline <- function(filename) {
.Call(`_vroom_has_trailing_newline`, filename)
}

vroom_rle <- function(input) {
.Call(`_vroom_vroom_rle`, input)
}

utctime_ <- function(year, month, day, hour, min, sec, psec) {
.Call(`_vroom_utctime_`, year, month, day, hour, min, sec, psec)
}
Expand Down Expand Up @@ -55,15 +67,3 @@ vroom_write_connection_ <- function(input, con, delim, eol, na_str, col_names, o
vroom_format_ <- function(input, delim, eol, na_str, col_names, append, options, num_threads, progress, buf_lines) {
.Call(`_vroom_vroom_format_`, input, delim, eol, na_str, col_names, append, options, num_threads, progress, buf_lines)
}

vroom_ <- function(inputs, delim, quote, trim_ws, escape_double, escape_backslash, comment, skip_empty_rows, skip, n_max, progress, col_names, col_types, col_select, name_repair, id, na, locale, guess_max, num_threads, altrep) {
.Call(`_vroom_vroom_`, inputs, delim, quote, trim_ws, escape_double, escape_backslash, comment, skip_empty_rows, skip, n_max, progress, col_names, col_types, col_select, name_repair, id, na, locale, guess_max, num_threads, altrep)
}

has_trailing_newline <- function(filename) {
.Call(`_vroom_has_trailing_newline`, filename)
}

vroom_rle <- function(input) {
.Call(`_vroom_vroom_rle`, input)
}
42 changes: 21 additions & 21 deletions src/cpp11.cpp
View file
Expand Up @@ -55,6 +55,27 @@ extern "C" SEXP _vroom_convert_connection(SEXP in_con, SEXP out_con, SEXP from,
return cpp11::as_sexp(convert_connection(cpp11::as_cpp<cpp11::decay_t<SEXP>>(in_con), cpp11::as_cpp<cpp11::decay_t<SEXP>>(out_con), cpp11::as_cpp<cpp11::decay_t<const std::string&>>(from), cpp11::as_cpp<cpp11::decay_t<const std::string&>>(to)));
END_CPP11
}
// vroom.cc
SEXP vroom_(const cpp11::list& inputs, SEXP delim, const char quote, bool trim_ws, bool escape_double, bool escape_backslash, const char* comment, const bool skip_empty_rows, size_t skip, ptrdiff_t n_max, bool progress, const cpp11::sexp& col_names, cpp11::sexp col_types, cpp11::sexp col_select, cpp11::sexp name_repair, SEXP id, const cpp11::strings& na, const cpp11::list& locale, ptrdiff_t guess_max, size_t num_threads, size_t altrep);
extern "C" SEXP _vroom_vroom_(SEXP inputs, SEXP delim, SEXP quote, SEXP trim_ws, SEXP escape_double, SEXP escape_backslash, SEXP comment, SEXP skip_empty_rows, SEXP skip, SEXP n_max, SEXP progress, SEXP col_names, SEXP col_types, SEXP col_select, SEXP name_repair, SEXP id, SEXP na, SEXP locale, SEXP guess_max, SEXP num_threads, SEXP altrep) {
BEGIN_CPP11
return cpp11::as_sexp(vroom_(cpp11::as_cpp<cpp11::decay_t<const cpp11::list&>>(inputs), cpp11::as_cpp<cpp11::decay_t<SEXP>>(delim), cpp11::as_cpp<cpp11::decay_t<const char>>(quote), cpp11::as_cpp<cpp11::decay_t<bool>>(trim_ws), cpp11::as_cpp<cpp11::decay_t<bool>>(escape_double), cpp11::as_cpp<cpp11::decay_t<bool>>(escape_backslash), cpp11::as_cpp<cpp11::decay_t<const char*>>(comment), cpp11::as_cpp<cpp11::decay_t<const bool>>(skip_empty_rows), cpp11::as_cpp<cpp11::decay_t<size_t>>(skip), cpp11::as_cpp<cpp11::decay_t<ptrdiff_t>>(n_max), cpp11::as_cpp<cpp11::decay_t<bool>>(progress), cpp11::as_cpp<cpp11::decay_t<const cpp11::sexp&>>(col_names), cpp11::as_cpp<cpp11::decay_t<cpp11::sexp>>(col_types), cpp11::as_cpp<cpp11::decay_t<cpp11::sexp>>(col_select), cpp11::as_cpp<cpp11::decay_t<cpp11::sexp>>(name_repair), cpp11::as_cpp<cpp11::decay_t<SEXP>>(id), cpp11::as_cpp<cpp11::decay_t<const cpp11::strings&>>(na), cpp11::as_cpp<cpp11::decay_t<const cpp11::list&>>(locale), cpp11::as_cpp<cpp11::decay_t<ptrdiff_t>>(guess_max), cpp11::as_cpp<cpp11::decay_t<size_t>>(num_threads), cpp11::as_cpp<cpp11::decay_t<size_t>>(altrep)));
END_CPP11
}
// vroom.cc
bool has_trailing_newline(const cpp11::strings& filename);
extern "C" SEXP _vroom_has_trailing_newline(SEXP filename) {
BEGIN_CPP11
return cpp11::as_sexp(has_trailing_newline(cpp11::as_cpp<cpp11::decay_t<const cpp11::strings&>>(filename)));
END_CPP11
}
// vroom.cc
SEXP vroom_rle(const cpp11::integers& input);
extern "C" SEXP _vroom_vroom_rle(SEXP input) {
BEGIN_CPP11
return cpp11::as_sexp(vroom_rle(cpp11::as_cpp<cpp11::decay_t<const cpp11::integers&>>(input)));
END_CPP11
}
// vroom_dttm.cc
cpp11::writable::doubles utctime_(const cpp11::integers& year, const cpp11::integers& month, const cpp11::integers& day, const cpp11::integers& hour, const cpp11::integers& min, const cpp11::integers& sec, const cpp11::doubles& psec);
extern "C" SEXP _vroom_utctime_(SEXP year, SEXP month, SEXP day, SEXP hour, SEXP min, SEXP sec, SEXP psec) {
Expand Down Expand Up @@ -106,27 +127,6 @@ extern "C" SEXP _vroom_vroom_format_(SEXP input, SEXP delim, SEXP eol, SEXP na_s
return cpp11::as_sexp(vroom_format_(cpp11::as_cpp<cpp11::decay_t<const cpp11::list&>>(input), cpp11::as_cpp<cpp11::decay_t<const char>>(delim), cpp11::as_cpp<cpp11::decay_t<const std::string&>>(eol), cpp11::as_cpp<cpp11::decay_t<const char*>>(na_str), cpp11::as_cpp<cpp11::decay_t<bool>>(col_names), cpp11::as_cpp<cpp11::decay_t<bool>>(append), cpp11::as_cpp<cpp11::decay_t<size_t>>(options), cpp11::as_cpp<cpp11::decay_t<size_t>>(num_threads), cpp11::as_cpp<cpp11::decay_t<bool>>(progress), cpp11::as_cpp<cpp11::decay_t<size_t>>(buf_lines)));
END_CPP11
}
// vroom.cc
SEXP vroom_(const cpp11::list& inputs, SEXP delim, const char quote, bool trim_ws, bool escape_double, bool escape_backslash, const char* comment, const bool skip_empty_rows, size_t skip, ptrdiff_t n_max, bool progress, const cpp11::sexp& col_names, cpp11::sexp col_types, cpp11::sexp col_select, cpp11::sexp name_repair, SEXP id, const cpp11::strings& na, const cpp11::list& locale, ptrdiff_t guess_max, size_t num_threads, size_t altrep);
extern "C" SEXP _vroom_vroom_(SEXP inputs, SEXP delim, SEXP quote, SEXP trim_ws, SEXP escape_double, SEXP escape_backslash, SEXP comment, SEXP skip_empty_rows, SEXP skip, SEXP n_max, SEXP progress, SEXP col_names, SEXP col_types, SEXP col_select, SEXP name_repair, SEXP id, SEXP na, SEXP locale, SEXP guess_max, SEXP num_threads, SEXP altrep) {
BEGIN_CPP11
return cpp11::as_sexp(vroom_(cpp11::as_cpp<cpp11::decay_t<const cpp11::list&>>(inputs), cpp11::as_cpp<cpp11::decay_t<SEXP>>(delim), cpp11::as_cpp<cpp11::decay_t<const char>>(quote), cpp11::as_cpp<cpp11::decay_t<bool>>(trim_ws), cpp11::as_cpp<cpp11::decay_t<bool>>(escape_double), cpp11::as_cpp<cpp11::decay_t<bool>>(escape_backslash), cpp11::as_cpp<cpp11::decay_t<const char*>>(comment), cpp11::as_cpp<cpp11::decay_t<const bool>>(skip_empty_rows), cpp11::as_cpp<cpp11::decay_t<size_t>>(skip), cpp11::as_cpp<cpp11::decay_t<ptrdiff_t>>(n_max), cpp11::as_cpp<cpp11::decay_t<bool>>(progress), cpp11::as_cpp<cpp11::decay_t<const cpp11::sexp&>>(col_names), cpp11::as_cpp<cpp11::decay_t<cpp11::sexp>>(col_types), cpp11::as_cpp<cpp11::decay_t<cpp11::sexp>>(col_select), cpp11::as_cpp<cpp11::decay_t<cpp11::sexp>>(name_repair), cpp11::as_cpp<cpp11::decay_t<SEXP>>(id), cpp11::as_cpp<cpp11::decay_t<const cpp11::strings&>>(na), cpp11::as_cpp<cpp11::decay_t<const cpp11::list&>>(locale), cpp11::as_cpp<cpp11::decay_t<ptrdiff_t>>(guess_max), cpp11::as_cpp<cpp11::decay_t<size_t>>(num_threads), cpp11::as_cpp<cpp11::decay_t<size_t>>(altrep)));
END_CPP11
}
// vroom.cc
bool has_trailing_newline(const cpp11::strings& filename);
extern "C" SEXP _vroom_has_trailing_newline(SEXP filename) {
BEGIN_CPP11
return cpp11::as_sexp(has_trailing_newline(cpp11::as_cpp<cpp11::decay_t<const cpp11::strings&>>(filename)));
END_CPP11
}
// vroom.cc
SEXP vroom_rle(const cpp11::integers& input);
extern "C" SEXP _vroom_vroom_rle(SEXP input) {
BEGIN_CPP11
return cpp11::as_sexp(vroom_rle(cpp11::as_cpp<cpp11::decay_t<const cpp11::integers&>>(input)));
END_CPP11
}

extern "C" {
static const R_CallMethodDef CallEntries[] = {
Expand Down
4 changes: 2 additions & 2 deletions src/vroom_write.cc
View file
Expand Up @@ -381,7 +381,7 @@ void vroom_write_out(
auto end = begin + num_lines;
futures[idx][t++] = std::async(
fill_buf,
input,
std::cref(input),

This comment has been minimized.

Sign in to view

Copy link
@MichaelChirico

MichaelChirico Oct 2, 2023 •
edited

Contributor

FWIW, this appears to have fixed some flakiness we observed in the vroom test suite. testthat.R was failing (very seriously -- process crash) in our CI system ~8% of the time, after updating to latest CRAN, there were no crashes in 1200 repetitions.

This comment has been minimized.

Sign in to view

Copy link
@jennybc

jennybc Oct 3, 2023

Author Member

I scrolled down here with dread in my heart, so it's a nice surprise to see this! Thanks for reporting.

This comment has been minimized.

Sign in to view

Copy link
@MichaelChirico

MichaelChirico Oct 3, 2023 •
edited

Contributor

Sorry for the unintentional cliffhanger!
delim,
eol,
na_str,
Expand Down Expand Up @@ -526,7 +526,7 @@ void vroom_write_out(
auto end = begin + num_lines;
futures[idx][t++] = std::async(
fill_buf,
input,
std::cref(input),
delim,
eol,
na_str,
Expand Down

0 comments on commit 2f96e7b

Please sign in to comment.