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

simplify replace ops #14

Closed
moodymudskipper opened this issue Aug 18, 2019 · 2 comments
Closed

simplify replace ops #14

moodymudskipper opened this issue Aug 18, 2019 · 2 comments

Comments

@moodymudskipper
Copy link
Owner

replace ops are built around x <- replace(x, cond, value) with specific cases for

  • factors
  • when the condition is never met.

I'm questioning both special cases.


First case was to counter this :

x <- factor(letters[1:3])
x[x == "b"] <- "Z"
#> Warning in `[<-.factor`(`*tmp*`, x == "b", value = "Z"): niveau de facteur
#> incorrect, NAs générés
x
#> [1] a    <NA> c   
#> Levels: a b c

so x == "b" <- "Z" would replace the levels smoothly.

I think it might have been a bad idea as we could simply do : levels(x) == "b" <- "Z" in that case with more consistency and less ambiguity in some corner cases.


Special case for when the condition was never met was designed so the following wouldn't alter x :

x <- 1:3
x < 0 <- "a"

but now I think it's good that x[x <0] <- "a" coerces to character even if the condition is never met, because of type stability. and our assignment functions would be more intuitive if they were all really simple shortcuts for cond(x) <- value equivalent to x[cond(x)] <- value.


@KKPMW do you see a problem with simplifying those ?

@karoliskoncevicius
Copy link
Collaborator

I agree with both of the points 100%. Unless I missed some corner case.

@moodymudskipper
Copy link
Owner Author

done with 19f10f9

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

No branches or pull requests

2 participants