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.

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

datefixR not working on Windows machines #85

Closed
atsyplenkov opened this issue Jun 25, 2023 · 4 comments
Closed

datefixR not working on Windows machines #85

atsyplenkov opened this issue Jun 25, 2023 · 4 comments
Assignees
Labels
bug Something isn't working

Comments

@atsyplenkov
Copy link
Contributor

All versions of datefixR (CRAN, ROpenSci, and Github) do not seem to be working on Windows machines running R 4.3.0 or R 4.3.1. I have tried on both Windows 10 (build 19044.3086) and a clean Windows 11 (build 22000.2057). Any function from the package freezes the R session.

At the same time, the Linux version works fine.

Windows 10 sessionInfo

R version 4.3.0 (2023-04-21 ucrt)
Platform: x86_64-w64-mingw32/x64 (64-bit)
Running under: Windows 10 x64 (build 19044)

Matrix products: default

locale:
[1] LC_COLLATE=English_New Zealand.utf8  LC_CTYPE=English_New Zealand.utf8
[3] LC_MONETARY=English_New Zealand.utf8 LC_NUMERIC=C
[5] LC_TIME=English_New Zealand.utf8

time zone: Pacific/Auckland
tzcode source: internal

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base

other attached packages:
[1] datefixR_1.4.1.9000

loaded via a namespace (and not attached):
[1] compiler_4.3.0  cli_3.6.1       tools_4.3.0     rstudioapi_0.14 Rcpp_1.0.10
[6] lifecycle_1.0.3 rlang_1.1.1

Ubuntu 20.04 sessionInfo

 > sessionInfo()
R version 4.3.0 (2023-04-21)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Ubuntu 20.04.6 LTS

Matrix products: default
BLAS:   /usr/lib/x86_64-linux-gnu/atlas/libblas.so.3.10.3
LAPACK: /usr/lib/x86_64-linux-gnu/atlas/liblapack.so.3.10.3;  LAPACK version 3.9.0

locale:
 [1] LC_CTYPE=C.UTF-8       LC_NUMERIC=C           LC_TIME=C.UTF-8
 [4] LC_COLLATE=C.UTF-8     LC_MONETARY=C.UTF-8    LC_MESSAGES=C.UTF-8
 [7] LC_PAPER=C.UTF-8       LC_NAME=C              LC_ADDRESS=C
[10] LC_TELEPHONE=C         LC_MEASUREMENT=C.UTF-8 LC_IDENTIFICATION=C

time zone: UTC
tzcode source: system (glibc)

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base

other attached packages:
[1] datefixR_1.4.1.9000

loaded via a namespace (and not attached):
 [1] compiler_4.3.0  magrittr_2.0.3  cli_3.6.1       tools_4.3.0
 [5] glue_1.6.2      rstudioapi_0.14 Rcpp_1.0.10     vctrs_0.6.3
 [9] stringi_1.7.12  stringr_1.5.0   lifecycle_1.0.3 rlang_1.1.1

@github-actions
Copy link
Contributor

Thank you for opening your first issue for datefixR!\n @nathansam will reply as soon as possible

@nathansam
Copy link
Collaborator

Thanks for opening the issue and giving session info. I have been able to recreate the same issue on my Windows 11 machine. Interestingly this problem hasn't been flagged by either GitHub actions checks or CRAN checks. I'll see what I can do, but it might take time to fix and might be because of a bug introduced by R or dependencies which needs to be fixed upstream.

@nathansam nathansam self-assigned this Jun 26, 2023
@nathansam nathansam added the bug Something isn't working label Jun 26, 2023
@nathansam
Copy link
Collaborator

The issue is the rm_ordinal_suffixes() internal function which uses C++ code. The bug seems to be introduced by Rtools 43 which uses new versions of GCC and MinGW . Will report the issue and look into a hack to get around it.

datefixR/src/code.cpp

Lines 30 to 43 in e9f0d79

// [[Rcpp::export]]
String rm_ordinal_suffixes(String date_) {
std::string date(date_);
date = std::regex_replace(date, std::regex("(\\d)(st,)"), "$1");
date = std::regex_replace(date, std::regex("(\\d)(nd,)"), "$1" );
date = std::regex_replace(date, std::regex("(\\d)(rd,)"), "$1" );
date = std::regex_replace(date, std::regex("(\\d)(th,)"), "$1" );
date = std::regex_replace(date, std::regex("(\\d)(st)"), "$1");
date = std::regex_replace(date, std::regex("(\\d)(nd)"), "$1" );
date = std::regex_replace(date, std::regex("(\\d)(rd)"), "$1" );
date = std::regex_replace(date, std::regex("(\\d)(th)"), "$1" );
String date2(date);
return date2;
}

@nathansam
Copy link
Collaborator

@atsyplenkov, I have pushed a fix to main by replacing some C++ code with R code. There shouldn't be any noticeable speed difference as the C++ implementation was in anticipation of converting the main for loop to C++ rather than to solve a specific bottleneck caused by the function. I will aim to submit a new release to CRAN over the weekend. If you could please let me know how you would like to be credited for your translation work in #84, that would speed up the process of submitting to CRAN.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants