From ff213c506eb5c92f88f7f2afd8065bcb8e0c1bbd Mon Sep 17 00:00:00 2001 From: Jan Marvin Garbuszus Date: Mon, 8 Nov 2021 22:53:40 +0100 Subject: [PATCH 1/4] fix #288 --- inst/extdata/gh_issue_288.xlsx | Bin 0 -> 9087 bytes src/write_file.cpp | 16 +++++++++++----- tests/testthat/test-date_time_conversion.R | 18 ++++++++++++++++++ 3 files changed, 29 insertions(+), 5 deletions(-) create mode 100644 inst/extdata/gh_issue_288.xlsx diff --git a/inst/extdata/gh_issue_288.xlsx b/inst/extdata/gh_issue_288.xlsx new file mode 100644 index 0000000000000000000000000000000000000000..f139461072240d27eeae4274da0c4bab04c562ff GIT binary patch literal 9087 zcmeHtg;!Kv`}U9$Lw61!C=G&2cY`3^ip0>}14EZ|H;9Bt3rGt{NJ%#gDV@?S@SCSz zpNGe9eSg6Fo>_Cw+UuPAT4(Ql-LbEIlogPW2?3}8GyniV3lQV+AGAdP0FY4t00ICS zqMn4Etuxrx*+3m)4|aON?rvjElY@-N_!fW&fBt{RfAI*EsQ1{mapE>$_(>H8_j>E0(x!Ov zYY_>CCg!yw#EYqIp`JD!LViRl3Lc=Ujs_T?}ted~~ZGAM|EIzVM*87kBUX_=kfE z=HVIZ-%jEuoREA3ACU&QlaK?@5ZtXf{_qnwJ4Y)cJ3Fi2ve{ogg8)xyaFqYtx~01k;Y~Yp(Cuh!apkQuzp*^9%fn*m9+3W9eJ$r(^ zghX)C%2E)9kL*EUy(fTT*MHQHgl4o`O6@O$LV&-swOzc6iJj&|P%j+U&H9FrckP^! zfh;69pHA|)ekAy@J;s;@rS7*WWcI5_Y$~DS*$=^^h5|6>`W8VEd%mligQ}c zN&Td;_sv=S>MPUdnM2mz<2}=rp04o)_)PzsG7w-S)fSuvV@Lo14*Ur?W&Tc`E8FaCYr-*8(OLtkgu z;;|vJLjjHQRdZ}<)=-r@W(KO;28_kfMFCf!(1aJ~ZH7hchqF%pDW)7Rd13-CHjcN| zgaaOWA!K4w#_=T3cwYIqSjYO3cWqrNuBbMi_ZmJ+Ipnv53?>4#i3D@Us1Lx#1F74v z=b7rxX?^P!eYuCzVUfl}6GMgjq7Rt9g`s%B13bxQl!DUEx7&cfi+bd z$>27{^J7{refO2~YZSGZHLxS1inEC`nSI_Z-L}-zZeF* z918l@v`E#qv#U-*7ng_GL9l@!E&vndE=(=l*p0+Y21R$-eR50d3lIC!hh3wH8C*9g zu@Vw6b5hCYi-#o`$i&Te9wWLw$-ln5N&(^*4@dYw?>je6Zz-$Mt@YS&jSrGAs(%a4EFHJ0T9g zd;gEl{Kj!7b1>N1>CXuHd*ji6;$vWRYgij6q0|}DjfA^r{6r8?LXC&6PGcR(=n{rH zlN@tKcx!3F0Kbr`&dpAg7f8@H_w#`4;b5Q~U(74v99h0@sog_gBe89_(>GRL*vR=deZ5Dg z=)aoIfDc>Ck?@ILruc1<^ShTjn}cn@9KWA`@Ai&Pzbyq1ZZpY-0IuhA*6BE7vREXA zq@|}0MpN=(l!Zjqy(ypcV-l$5!dA53iVjz8yc4w(BD6uG7v^_cqAZZc?_2b;Ii_oN}W$fQh9P!mK*Z7Yb*4}4mDUnjLrazGy$zs>t zogmk$$v?#MErjU5AGL5RQiGuSE|a=ix;TI*8mz`AB=eCfLF?dJmJ|8@E`dX>HD}u@ z-BOR-u?~zUG*=Z*Lkx;bCY~k>H6rLhy|4|eY-(nTspoxJyB6tjEO*DQ|G?K&6CDn89&-H z5b_CoDdgk$V}JUfJu3<%#P5Bco4Bet+X-1xf?n*J`FLMHx?JAm;Q;yIbt;hDZsTPA zI6PW6q*C;g4bnn2pc5Yc7QuF- zniBI&8iM~)KSF<&x-%rZdWQE*e};^xr4YCBsK)O77d5BkZ~ET?S0rh#BS5(LDX!6~ zgJlAbiS-Y&#m^Wm4apzNMV~f_rYXYm$!4Z1dYILmd0b_J|6 zE@ZPe#AMeL`|87Im+tKnnn@{}rM+iAMS=im!-mn>jxIg50lB%EuE`ah^7%#>BP zM^F4r_Ozdqk9MZ2mJq{pv~a=&$2e8OsJi{0t9!?;#(Se@>uD+8?v+Df#P$;%nq`f1 zMjFNA;vX;cipj@;Odfjawq#J=&re7*bP2Yok;n11Aw*@ss_#QshSI38?={g`by6H& z+3ayl)G;G69t%gpMdS|^jllTQ2%yd8BSH{g@sp~lTJ2p{CCB7qHf`IbrrLv%yy-U( zx%J&pXOsH$3|1ML{sI+)G8x8BDQI9_|Se?~lc?v3{71oqLvoTMR0i%`A>SK^zCvU%+2#5Jb9`g(tfwBC**^MIZY6*+2aSA zl(~5YqI6=H;%r^Ql(?qb6se!Pa4wY8NDEErDZ72FKeFq{`+;K9fwg=)t$7SVBq3x$ z+Vvx~O+u08H%P}LVP0(!my%yD^R;@?PKC>6F9eLHEm-uP`9ZRxd|S(B22QC87FOFG zMn=b2kCjKE{UoaJ;8F=slWmZlB`(qbnIzxV*S4a$harI{kMEUjeH0RB3T!Zpk3$81 zH~Dd~&YyI=##T#`w(AXguG5 zHP=Rcks}qomL~+W>@2TCQ^HcFlNl~WcMpUXHIes(tsy})MI6&@NEZH8$5SC~J=m~U zBRSTss+zSVFQr?tAUc~$lR>Or@$zRGoo|TEZX92-H#c7jW>qL)de*qI%=s45T0GA$ zV2^$f8Q~BC>0N6^TfV+p++cHGOuSO8#7WgNh&7EWZDtk3UDDns4 zOPWG>2E+QZ7;%PJgMY6=@)BRzrE?Mn?>*cV(QbZ^ieE{OSoDrb^<7!om=a9Nj*>G} z!-vMW!DRI2D#{NxN6D&om98Q>8uWR+T?tyBj3~sd=2VJLseexTC~@1r*v9{}pvgRC zE=*NJIh#mf-*5b!%j=fCY*(!OiprQ6L3|v2UPq4&z3_-CzN&eMNUdDe@_u1Z;dERB zR|IkO3_S|Emyb04Du!F&yS@d)*fGH;iOE(vD!n}V_MYZCq&-l`M-A9eB%tu>mHg5D zPnnG^3U4T0Y>&JIL~T^A@Oc(GiV-8rT9lBd%IW3m^L!PrK_%GIw8h{Y$R~9XGuT8l z>6TFyj6;HQ${CMvkJOD>Mu?ZTt;UZmsIMjQHy_C_UkZ5SBeMD#ujrZwu;!8NmrAMh z@JF*SpLg1?YpV8iWOIiM3~A<{#Zw-RVQeYUlu}N82_i)V(u$hN8p~FLrNNwH*XQ4q zTMsZAa@woBKe*dr2_YLkhKa(EM%{B25nk{r*_g>2x5{H~VJ&>0{V+}hwX3RDu6)eS zJ7*~s)Vu2-BT_)C@$9wdWT8PX_h~a_BG6-AYOiQL7*|$g$9?Y0L046jP@58sj!(4I zmE&U8?)R&WCpzuklEw0r-^uF@A6V__FHo3`aY7YJK1j@{i|*$FvsT@@2^~vQ7X{wS zwt#nXrP6~jd&lh@G5d^sijwxyIvObECLDNCm^I4dp0)`C!_kE!MJoJx93Q!RtqsCz zUL$99)|M6erwRn@?%H6vVR@?8~?ZmpFA*#|mPw0|o*YCOiThI2)b90M&6aQl4(kLQwy=+;|YYE&HZFI-W zXt`fkuCF*o{TSiQS40K?NdHy2{;XLO5)`A;IdOyc5-&ll3na4Qt^~+BQjs}p%7t5} z%hr7WP0sw&mF)#L>ooK6ISCK6%AWnc?K7(Gtic9jnR2Dn=EZo%Z$J+V=5ZdB+XJtw z^2NUF4VK|lY7B3Lt}HE($jU?GvoaJ>>A-oSMk&flUe`(#HhmM+jm<2WHd{a~-4fkcet!ArN|MQ;6Y~uG|zUWN2s^tzrnB{<=&f;L}J| z^805~##dk<#T9HH*~MXH)FE-@AQqcrQ>Kw(Ou&U>Y?2sCZf}vs zo}_Xq?0gDRHrP(|h_v8;ZhA}8wIO{Ub-0DZkYZe5LX67aeW@=>hXJp27${6_=WY>G z()3-|1IMrAV7LNC+GuMoD0qgl&&g>_ghKhU)E}oahX|ukBHw&*rz|Zn&=%xB4uW&% z-)n!g;79>^_?pNN&M-WnK^K2YK`9mH=75Y=82E zk13MKw^(=gd=XDwlO@rp~2=o61!}>zSvP@nY>)1V5U+q14h?3gdh7 zfUjyyDV!}Yj1RSJ)dU63J{8xwSbbhoL_aLsz7HNpsrCF6AQ9$sPta}%lud)g@kRPt zZI!Lwhf$O9{UZiiTEh&&KrVUGrkjlgk?lQ{uJB9q1JF{=TlCxn1}*L-^cbA^^(?yE zU#bj)i!xPha00Rt0RRvG2ygaqdBGX%s0Ma+{!M4#xYK^bZe(G*quwt+t4u1yU9r27 zk4-r1tdhs#umOxOoYP3nbCn+nr01@4c;u{#7SR(UDf!O-oLpEonf=8nLW3)9$36Sp z^?dHBr0j@A>GSWapB4s%_;}7@pik+bVqwT+yDW~4{=I_Y`d|2pi;NbL@*_twPLChX zlEBjO#~SrznUOBnr4tXfksSIx8dp4lo*_SCl zRF7|6?Y8IMW*+8EWpLC{FA@bU;W2Ywl$kN=yc|?m52xI&3GPkQP4k>3(Z*)5Q2o_roB0I; z?PqiH-G&wQ<8Yy?m96QncONGXa3)F)sF~gLF^UfpE3_{7OWz+#?cbnZ^|8B7A*q{f zRxT9SF6Im}wo{*}0nu(qHtjjy;LSYf5|&;$y9nfe?oyfYge=FGkp|;6;x8_JIW26z zj6?Hc#M5DL8sNb-NZh~2Awzrn|Ais=ko|dN#EIF>Z~}w&kWN9sISRfxexP58ku1+~ zxe-E~tC?cHadsvB>yp@=$>^HlF$+&vJrKH>o5Wm53%y5Tt=xPoj3NnQi$!TKU z!gbv_GX_0@<+8D_JS9R$_074MjpMVJ9t2`aymEXt6#n~o)|6bj><(d~EMvONsaZ zmv{irElP0lgQ+oBlSE?%K{SwHFYqJPc|P|#bUE=NPiuO6ih&@CHKQntU2w?@)TiAe&l2>m>D{XRr)ehff5!4p_4#KFe;4JS)!9!B0N@I~ z6!=%IcGvuFZv6=Wg^R%O_rH@_@3w!>w0A-O8L59l0ssM2e|x7tK_3Y>!g01*Cr M0H?78&2O#$1IuxEFaQ7m literal 0 HcmV?d00001 diff --git a/src/write_file.cpp b/src/write_file.cpp index 6df31be1..23334b0d 100644 --- a/src/write_file.cpp +++ b/src/write_file.cpp @@ -1,5 +1,6 @@ #include "openxlsx.h" +#include //' @import Rcpp @@ -225,16 +226,21 @@ SEXP buildMatrixMixed(CharacterVector v, if(!notNAElements[ri]){ datetmp[ri] = NA_REAL; //IF TRUE, TRUE else FALSE }else{ - // dt_str = as(m(ri,i)); dt_str = m(ri,i); + try{ - datetmp[ri] = Rcpp::Date(atoi(dt_str.substr(5,2).c_str()), atoi(dt_str.substr(8,2).c_str()), atoi(dt_str.substr(0,4).c_str()) ); - }catch(...) { + // Some values are incorrectly detected as dates. This regex determines if they are numerics. + // If so, they are converted to Dates. + Rcpp::Function cTD("convertToDate"); + if (std::regex_match( dt_str, std::regex( ( "((\\+|-)?[[:digit:]]+)(\\.(([[:digit:]]+)?))?" ) ) )) { + datetmp[ri] = as(cTD(dt_str)); + } else { + datetmp[ri] = Rcpp::Date(atoi(dt_str.substr(5,2).c_str()), atoi(dt_str.substr(8,2).c_str()), atoi(dt_str.substr(0,4).c_str()) ); + } + } catch(...) { Rcpp::Rcerr << "Error reading date:\n" << dt_str << "\nrow: " << ri+1 << "\ncol: " << i+1 << "\n"; throw; } - //datetmp[ri] = Date(atoi(m(ri,i)) - originAdj); - //datetmp[ri] = Date(as(m(ri,i))); } } diff --git a/tests/testthat/test-date_time_conversion.R b/tests/testthat/test-date_time_conversion.R index ab2e4e11..23307cae 100644 --- a/tests/testthat/test-date_time_conversion.R +++ b/tests/testthat/test-date_time_conversion.R @@ -33,3 +33,21 @@ test_that("convert to datetime", { x_datetime <- convertToDateTime(x, tx = "UTC") attr(x_datetime, "tzone") <- "UTC" }) + + + +test_that("read.xlsx detectDates", { + + xlsxFile <- system.file("extdata", "gh_issue_288.xlsx", package = "openxlsx") + + ref_dat <- data.frame(Date = c(as.Date(c("2021-10-20", "2021-11-03")))) + ref_num <- data.frame(Date = c(44489.4, 44503.0)) + + tst_dat <- read.xlsx(xlsxFile, detectDates = TRUE) + tst_num <- read.xlsx(xlsxFile, detectDates = FALSE) + + expect_equal(ref_dat, ref_dat) + expect_equal(ref_num, ref_num) + +}) + \ No newline at end of file From 52d3197c64c897abc1f9ef935f32b5b9241175fe Mon Sep 17 00:00:00 2001 From: Jan Marvin Garbuszus Date: Mon, 8 Nov 2021 23:32:53 +0100 Subject: [PATCH 2/4] remove blanks --- tests/testthat/test-date_time_conversion.R | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tests/testthat/test-date_time_conversion.R b/tests/testthat/test-date_time_conversion.R index 23307cae..4f41148c 100644 --- a/tests/testthat/test-date_time_conversion.R +++ b/tests/testthat/test-date_time_conversion.R @@ -1,9 +1,7 @@ - context("Date/Time Conversions") - test_that("convert to date", { dates <- as.Date("2015-02-07") + -10:10 origin <- 25569L @@ -18,7 +16,6 @@ test_that("convert to date", { }) - test_that("convert to datetime", { x <- 43037 + 2 / 1440 expect_equal(object = convertToDateTime(x, tx = Sys.timezone()), expected = as.POSIXct("2017-10-29 00:02:00", tz = Sys.timezone())) @@ -35,7 +32,6 @@ test_that("convert to datetime", { }) - test_that("read.xlsx detectDates", { xlsxFile <- system.file("extdata", "gh_issue_288.xlsx", package = "openxlsx") @@ -50,4 +46,3 @@ test_that("read.xlsx detectDates", { expect_equal(ref_num, ref_num) }) - \ No newline at end of file From b93d7868b42d5c2801dc40bcdc364a6f4e37f1e0 Mon Sep 17 00:00:00 2001 From: Jan Marvin Garbuszus Date: Mon, 8 Nov 2021 23:58:49 +0100 Subject: [PATCH 3/4] fix typo in test --- tests/testthat/test-date_time_conversion.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/testthat/test-date_time_conversion.R b/tests/testthat/test-date_time_conversion.R index 4f41148c..185fb013 100644 --- a/tests/testthat/test-date_time_conversion.R +++ b/tests/testthat/test-date_time_conversion.R @@ -42,7 +42,7 @@ test_that("read.xlsx detectDates", { tst_dat <- read.xlsx(xlsxFile, detectDates = TRUE) tst_num <- read.xlsx(xlsxFile, detectDates = FALSE) - expect_equal(ref_dat, ref_dat) - expect_equal(ref_num, ref_num) + expect_equal(ref_dat, tst_dat) + expect_equal(ref_num, tst_num) }) From 5b5340f6a436d458e918f1410ec89dc94d502667 Mon Sep 17 00:00:00 2001 From: Jan Marvin Garbuszus Date: Wed, 15 Dec 2021 21:57:12 +0100 Subject: [PATCH 4/4] add NEWS entry and bump development version --- DESCRIPTION | 2 +- NEWS.md | 6 ++++++ src/write_file.cpp | 4 ++-- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 7caeae8a..15e70430 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,7 +1,7 @@ Type: Package Package: openxlsx Title: Read, Write and Edit xlsx Files -Version: 4.2.5 +Version: 4.2.5.1 Date: 2021-12-13 Authors@R: c(person(given = "Philipp", diff --git a/NEWS.md b/NEWS.md index 3ecf0dde..39f0aa02 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,9 @@ +# openxlsx (development version) + +## Improvements + +* Improve detectDates ([#288](https://github.com/ycphs/openxlsx/issues/288)) + # openxlsx 4.2.5 ## Fixes diff --git a/src/write_file.cpp b/src/write_file.cpp index 23334b0d..fad12d62 100644 --- a/src/write_file.cpp +++ b/src/write_file.cpp @@ -198,7 +198,8 @@ SEXP buildMatrixMixed(CharacterVector v, for(int i = 0;i < k; i++) m(rowInd[i], colInd[i]) = v[i]; - + // convert numerics to dates + Rcpp::Function cTD("convertToDate"); // this will be the return data.frame List dfList(nCols); @@ -231,7 +232,6 @@ SEXP buildMatrixMixed(CharacterVector v, try{ // Some values are incorrectly detected as dates. This regex determines if they are numerics. // If so, they are converted to Dates. - Rcpp::Function cTD("convertToDate"); if (std::regex_match( dt_str, std::regex( ( "((\\+|-)?[[:digit:]]+)(\\.(([[:digit:]]+)?))?" ) ) )) { datetmp[ri] = as(cTD(dt_str)); } else {