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

Create addNAstr instead of overriding base::addNA #18

Closed
juba opened this issue Nov 7, 2013 · 3 comments
Closed

Create addNAstr instead of overriding base::addNA #18

juba opened this issue Nov 7, 2013 · 3 comments
Labels
Milestone

Comments

@juba
Copy link
Owner

juba commented Nov 7, 2013

The new addNA function (see #12) adds an interesting functionality, but I'm not sure it's a good way to implement it because :

  • it overrides a base function, which I'm not very happy with
  • this overriding generates a warning each time questionr is loaded
  • it duplicates the code of the base function

So I wonder if it would not be better, instead of overriding and adding an argument to addNA, to create a new addNAstr function (or whatever better name we could find) which would be something like (not tested) :

addNAstr <- function(..., value=NULL) {
   x <- addNA(...)
   s <- ifelse(is.character(value), value, "NA")
   levels(x)[is.na(levels(x))] <- s
   x
}

What do you think ?

@larmarange
Copy link
Contributor

Well. I'm ok with both options.

@juba
Copy link
Owner Author

juba commented Nov 7, 2013

In fact, I'm currently fighting with the fact that one of the imported package in questionr is overriding another base function (see #19), so I would be very inclined not to do it. But I'm not objective at all right now :-)

@larmarange
Copy link
Contributor

Well you can change the name of the function. It's not a major concern for
me.

Joseph Larmarange
Le 7 nov. 2013 15:58, "Julien" notifications@github.com a écrit :

In fact, I'm currently fighting with the fact that one of the imported
package in questionr is overriding another base function (see #19#19),
so I would be very inclined not to do it. But I'm not objective at all
right now :-)


Reply to this email directly or view it on GitHubhttps://github.com//issues/18#issuecomment-27972105
.

@juba juba closed this as completed in 3baaea9 Nov 8, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants