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

replace_with_na throws error when replace list elements don't exist #160

Closed
m-sostero opened this issue May 4, 2018 · 3 comments
Closed
Milestone

Comments

@m-sostero
Copy link

replace_with_na throws an error when at least one of the replace list elements doesn't exist in the data, even if other elements do exist.

dat_ms <- tibble::tribble(
  ~x,  ~y,    ~z,
  1,   "A",   -100,
  3,   "N/A", -99,
  NA,  NA,    -98,
  -99, "E",   -101,
  -98, "F",   -1)

replace_with_na(
  dat_ms,
  replace = list(
    x = -99, # dat_ms$x exists
    w = -99  # dat_ms$w does not
  )
)

This gives

> Error in `[[<-.data.frame`(`*tmp*`, var, value = logical(0)) : 
  replacement has 0 rows, data has 5

I think the function return the data with all possible replacements, and throw an informative warning about replacement variables that do not exist in the data.

I will work on a PR for this.

@njtierney
Copy link
Owner

Ah, thank you for finding this, @m-sostero !

If you can work on a PR that would be fantastic!

@njtierney njtierney added this to the V0.4.0 milestone Jun 5, 2018
@njtierney njtierney removed the V0.4.0 label Jun 5, 2018
@njtierney njtierney modified the milestones: V0.4.0, V0.5.0 Sep 3, 2018
@njtierney njtierney modified the milestones: V0.5.0, V0.6.0 May 19, 2020
@michael-dewar
Copy link

I don't know how to write a method, but an R wrapper like this should work:

safe_replace_with_na <- function(data, replace, ...){
	new_replace <- replace[names(replace) %in% names(data)]

	missing <- setdiff(names(replace), names(new_replace))
	if(length(missing) >0){
		warning(paste("Missing from data:", paste(missing, collapse = ", ")))
	}

	data %>% naniar::replace_with_na(new_replace, ...)
}

@njtierney
Copy link
Owner

Thanks for this, @michael-dewar! I've rolled this into replace_with_na and noted you in the NEWS file.

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

No branches or pull requests

3 participants