-
Notifications
You must be signed in to change notification settings - Fork 195
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
Support multiple na values #56
Conversation
@@ -53,7 +53,7 @@ parse_ref <- function(ref) { | |||
.Call('readxl_parse_ref', PACKAGE = 'readxl', ref) | |||
} | |||
|
|||
xlsx_col_types <- function(path, sheet = 0L, na = "", nskip = 0L, n = 100L) { | |||
xlsx_col_types <- function(path, sheet = 0L, na = as.character( c()), nskip = 0L, n = 100L) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A more compact form is character()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah - by the looks the Rcpp attributes compilation changed in 0.11.1. Instead of CharacterVector::create() to create a default, you should now use CharacterVector().
Fixed.
Looks great apart from that one tiny problem |
Rebased + updated. |
@jmarshallnz @hadley any activity here? Wondering whether to patch up #136 or not. |
Looks good - just needs a bullet point in NEWS. @jennybc can you please merge when ready? (or add the bullet point if @jmarshallnz has moved on to other things) |
I can do it on the 24th. On holiday ATM
…On Thu, 19 Jan 2017 at 6:06 AM, Hadley Wickham ***@***.***> wrote:
Looks good - just needs a bullet point in NEWS.
@jennybc <https://github.com/jennybc> can you please merge when ready?
(or add the bullet point if @jmarshallnz <https://github.com/jmarshallnz>
has moved on to other things)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#56 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAY0KzeDLPeuQUSOF9wuHYJnxuYs3OsIks5rTkaFgaJpZM4D90xi>
.
|
Takes care of #13, and fixes a bug where NA = "1" didn't work for .xls sheets.