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

Fix Excel false leapyear issue #59

Merged
merged 1 commit into from
Jun 25, 2021
Merged

Fix Excel false leapyear issue #59

merged 1 commit into from
Jun 25, 2021

Conversation

jmcnamara
Copy link
Contributor

Fix issue where Excel treats 1900 as a leapyear and add an
extra day.

Issue #58

Fix issue where Excel treats 1900 as a leapyear and add an
extra day.

Issue #58
@jeroen jeroen merged commit f1ba9c6 into ropensci:master Jun 25, 2021
@jeroen
Copy link
Member

jeroen commented Jun 25, 2021

Thank you

@jmcnamara
Copy link
Contributor Author

After doing some more testing the solution is incorrect for hours on 1900/02/28. The solution should be (relative to master):

diff --git a/src/write_xlsx.c b/src/write_xlsx.c
index 4a3c15b..afb0656 100644
--- a/src/write_xlsx.c
+++ b/src/write_xlsx.c
@@ -155,7 +155,7 @@ SEXP C_write_data_frame_list(SEXP df_list, SEXP file, SEXP col_names, SEXP forma
           double val = REAL(col)[i];
           if(R_FINITE(val))
             val = 25568.0 + val / (24*60*60);
-            if(val > 59.0)
+            if(val >= 60.0)
               val = val + 1.0;
             assert_lxw(worksheet_write_number(sheet, cursor, j, val , NULL));
         }; continue;

Note, I'm adding a function to libxlsxwriter to do this conversion directly (and with unit tests) so that you, and other users, will be able to avoid issues like this in the future.

I can submit another PR if you wish.

@jeroen
Copy link
Member

jeroen commented Jun 28, 2021

OK sounds good

jeroen pushed a commit that referenced this pull request Jun 29, 2021
Fix leapday cutoff date from previous commit.

Issue #58
Previous PR #59
@jeroen
Copy link
Member

jeroen commented Dec 29, 2022

Sorry for the late comment, but did you intend to put parentheses around this block as fixed in 876b74d ?

@jmcnamara
Copy link
Contributor Author

Sorry for the late comment, but did you intend to put parentheses around this block as fixed in 876b74d ?

Yes. Apologies. That was a bad miss on my part.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants