moodymudskipper / inops

Infix Operators for Detection, Subsetting and Replacement
GNU General Public License v3.0
40 stars 0 forks source link

simplify replace ops #14

Closed moodymudskipper closed 4 years ago

moodymudskipper commented 4 years ago

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

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 commented 4 years ago

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

moodymudskipper commented 4 years ago

done with https://github.com/moodymudskipper/rangeops/commit/19f10f96334e9640fe4320c682c2ca1278af45e3