refactor: in $fill_nan(), rename the only arg value (#1198)

NEWS.md

@@ -22,7 +22,9 @@
   For now, `future = FALSE` can be replaced by `compat_level = FALSE` (#1183).
 - In `$scan_parquet()` and `$read_parquet()`, the default value of
   `hive_partitioning` is now `NULL` (#1189).
-- In `$dt$epoch()`, the argument `tu` is renamed to `time_unit` (#1196). 
+- In `$dt$epoch()`, the argument `tu` is renamed to `time_unit` (#1196).
+- In `$fill_nan()` for `DataFrame`, `LazyFrame` and `Expr`, the argument is
+  renamed `value` (#1198).
 
 ### New features
 

R/dataframe__frame.R

@@ -1333,19 +1333,17 @@ DataFrame_reverse = function() {
   self$lazy()$reverse()$collect()
 }
 
-#' @title Fill `NaN`
-#' @description Fill `NaN` values by an Expression evaluation.
-#' @keywords DataFrame
-#' @param fill_value Value to fill `NaN` with.
+#' @inherit Expr_fill_nan title params
+#'
 #' @return DataFrame
 #' @examples
 #' df = pl$DataFrame(
 #'   a = c(1.5, 2, NaN, 4),
 #'   b = c(1.5, NaN, NaN, 4)
 #' )
 #' df$fill_nan(99)
-DataFrame_fill_nan = function(fill_value) {
-  self$lazy()$fill_nan(fill_value)$collect()
+DataFrame_fill_nan = function(value) {
+  self$lazy()$fill_nan(value)$collect()
 }
 
 #' @title Fill nulls

R/expr__expr.R

@@ -1656,9 +1656,10 @@ Expr_forward_fill = function(limit = NULL) {
 }
 
 
-#' Fill NaN
+#' Fill floating point NaN value with a fill value
+#'
+#' @param value Value used to fill `NaN` values.
 #'
-#' @param expr Expr or something coercible in an Expr
 #' @return Expr
 #' @examples
 #' pl$DataFrame(a = c(NaN, 1, NaN, 2, NA))$
@@ -1667,8 +1668,9 @@ Expr_forward_fill = function(limit = NULL) {
 #'   # implicit coercion to string
 #'   string = pl$col("a")$fill_nan("invalid")
 #' )
-Expr_fill_nan = function(expr = NULL) {
-  .pr$Expr$fill_nan(self, wrap_e(expr))
+Expr_fill_nan = function(value = NULL) {
+  .pr$Expr$fill_nan(self, value) |>
+    unwrap("in $fill_nan():")
 }
 
 

R/extendr-wrappers.R

@@ -556,7 +556,7 @@ RPolarsExpr$fill_null <- function(expr) .Call(wrap__RPolarsExpr__fill_null, self
 
 RPolarsExpr$fill_null_with_strategy <- function(strategy, limit) .Call(wrap__RPolarsExpr__fill_null_with_strategy, self, strategy, limit)
 
-RPolarsExpr$fill_nan <- function(expr) .Call(wrap__RPolarsExpr__fill_nan, self, expr)
+RPolarsExpr$fill_nan <- function(value) .Call(wrap__RPolarsExpr__fill_nan, self, value)
 
 RPolarsExpr$reverse <- function() .Call(wrap__RPolarsExpr__reverse, self)
 
@@ -1228,9 +1228,9 @@ RPolarsLazyFrame$reverse <- function() .Call(wrap__RPolarsLazyFrame__reverse, se
 
 RPolarsLazyFrame$drop <- function(columns) .Call(wrap__RPolarsLazyFrame__drop, self, columns)
 
-RPolarsLazyFrame$fill_nan <- function(fill_value) .Call(wrap__RPolarsLazyFrame__fill_nan, self, fill_value)
+RPolarsLazyFrame$fill_nan <- function(value) .Call(wrap__RPolarsLazyFrame__fill_nan, self, value)
 
-RPolarsLazyFrame$fill_null <- function(fill_value) .Call(wrap__RPolarsLazyFrame__fill_null, self, fill_value)
+RPolarsLazyFrame$fill_null <- function(value) .Call(wrap__RPolarsLazyFrame__fill_null, self, value)
 
 RPolarsLazyFrame$slice <- function(offset, length) .Call(wrap__RPolarsLazyFrame__slice, self, offset, length)
 

R/lazyframe__lazy.R

@@ -1078,17 +1078,17 @@ LazyFrame_quantile = function(quantile, interpolation = "nearest") {
   unwrap(.pr$LazyFrame$quantile(self, wrap_e_result(quantile), interpolation), "in $quantile():")
 }
 
-#' @inherit DataFrame_fill_nan title description params
-#' @keywords LazyFrame
+#' @inherit Expr_fill_nan title params
+#'
 #' @return LazyFrame
 #' @examples
 #' df = pl$LazyFrame(
 #'   a = c(1.5, 2, NaN, 4),
 #'   b = c(1.5, NaN, NaN, 4)
 #' )
 #' df$fill_nan(99)$collect()
-LazyFrame_fill_nan = function(fill_value) {
-  unwrap(.pr$LazyFrame$fill_nan(self, wrap_e_result(fill_value)), "in $fill_nan():")
+LazyFrame_fill_nan = function(value) {
+  unwrap(.pr$LazyFrame$fill_nan(self, value), "in $fill_nan():")
 }
 
 #' @inherit DataFrame_fill_null title description params

src/rust/src/lazy/dataframe.rs

@@ -274,20 +274,12 @@ impl RPolarsLazyFrame {
         Ok(self.0.clone().drop(robj_to!(Vec, String, columns)?).into())
     }
 
-    fn fill_nan(&self, fill_value: Robj) -> RResult<Self> {
-        Ok(self
-            .0
-            .clone()
-            .fill_nan(robj_to!(Expr, fill_value)?.0)
-            .into())
+    fn fill_nan(&self, value: Robj) -> RResult<Self> {
+        Ok(self.0.clone().fill_nan(robj_to!(PLExpr, value)?).into())
     }
 
-    fn fill_null(&self, fill_value: Robj) -> RResult<Self> {
-        Ok(self
-            .0
-            .clone()
-            .fill_null(robj_to!(Expr, fill_value)?.0)
-            .into())
+    fn fill_null(&self, value: Robj) -> RResult<Self> {
+        Ok(self.0.clone().fill_null(robj_to!(Expr, value)?.0).into())
     }
 
     fn slice(&self, offset: Robj, length: Robj) -> RResult<Self> {

src/rust/src/lazy/dsl.rs

@@ -351,8 +351,8 @@ impl RPolarsExpr {
         Ok(self.0.clone().fill_null_with_strategy(strat).into())
     }
 
-    pub fn fill_nan(&self, expr: &RPolarsExpr) -> Self {
-        self.0.clone().fill_nan(expr.0.clone()).into()
+    pub fn fill_nan(&self, value: Robj) -> RResult<Self> {
+        Ok(self.0.clone().fill_nan(robj_to!(PLExpr, value)?).into())
     }
 
     pub fn reverse(&self) -> Self {

tests/testthat/test-expr_expr.R

@@ -1079,10 +1079,10 @@ test_that("shift", {
 })
 
 
-test_that("fill_null  + forward backward _fill + fill_nan", {
+test_that("fill_null  + forward backward _fill", {
   l = list(a = c(1L, rep(NA_integer_, 3L), 10))
 
-  # fiil value
+  # fill value
   expect_identical(
     pl$DataFrame(l)$select(pl$col("a")$fill_null(42L))$to_list()$a,
     l$a |> (\(x) {
@@ -1091,7 +1091,7 @@ test_that("fill_null  + forward backward _fill + fill_nan", {
     })()
   )
 
-  # forwarnd
+  # forward
 
   R_fill_fwd = \(x, lim = Inf) {
     last_seen = NA
@@ -1174,8 +1174,9 @@ test_that("fill_null  + forward backward _fill + fill_nan", {
       a_bfill_NULL = R_fill_bwd(l$a)
     )
   )
+})
 
-  # Fill NaN
+test_that("fill_nan() works", {
   R_replace_nan = \(x, y) {
     x[is.nan(x)] = y
     x