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

consider na_string() default for na.strings #473

Closed
jmbarbone opened this issue Dec 13, 2022 · 0 comments · Fixed by #474
Closed

consider na_string() default for na.strings #473

jmbarbone opened this issue Dec 13, 2022 · 0 comments · Fixed by #474
Labels
internal 🔧 question ❓ Further information is requested

Comments

@jmbarbone
Copy link
Collaborator

waiver function for explicit na.strings setting

na_string <- function() {
  structure("na_string", class = "openxlsx2_waiver")
}

is_na_string <- function(x) {
  is_waiver(x) && isTRUE(x == "na_string")
}

@JanMarvin , I did some testing and was able to implement this without having to update any of the tests. I'm pretty sure my poor handling of these is what's wrong with #471 .

This would help with using explicit values (definitely my preference; and we only have to check this once) but also (with another small internal adjustment) allow users to set their default strings value in options().

If we would want to use

foo <- function(
    ...
    na.strings = getOption("openxlsx2.na.strings", na_strings())
    ...
) {
  ...
}

We could allow zero length vectors (character()) to replace the NULL behavior within write_data2(); which currently errors through txt_to_is(character()). Explicitly setting na.strings = NULL would have the same effect; as would not setting it.

@jmbarbone jmbarbone added question ❓ Further information is requested internal 🔧 labels Dec 13, 2022
@jmbarbone jmbarbone changed the title consider na_strings() default for na.strings consider na_string() default for na.strings Dec 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal 🔧 question ❓ Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant