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

[R/write] fixes for NA, NaN, and Inf. Closes #254 #255

Merged
merged 3 commits into from
Jun 29, 2022
Merged

Conversation

JanMarvin
Copy link
Owner

This fixes the read/write of NA, NaN and Inf. I'm not sure yet if it's a good idea to always convert #NUM! and/or #VALUE! and maybe more openxml exceptions to R's NaN, NA or Inf. All these exceptions have special meaning in R and must not match their openxml counterparts. I am open to arguments as to why we should or should not do this.

May want to change "#VALUE!" and "#NUM!" to NaN and Inf in the future, but no consensus reached yet
@@ -503,7 +514,7 @@ write_data2 <-function(wb, sheet, data, name = NULL,
}

sel <- which(dc == openxlsx2_celltype[["character"]]) # character
for (i in sel) {
if (length(sel)) {
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks pretty stupid to be honest 🤦

## data
a <- data.frame(
A <- data.frame(
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this test was copied over from openxlsx (at least I hope nobody here uses c as object name) and shortened when we did not support all features. Now that we support more features, we might want to add the missing pieces.

@JanMarvin JanMarvin merged commit 9c3e2ff into main Jun 29, 2022
@JanMarvin JanMarvin deleted the write_nan_inf branch June 29, 2022 08:09
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.

None yet

1 participant