[GitHub] [spark] HyukjinKwon commented on a diff in pull request #37526: [WIP][SPARK-40087][R][SQL] Support multiple "Column" drop in R

[GitBox <gi...@apache.org>]

HyukjinKwon commented on code in PR #37526:
URL: https://github.com/apache/spark/pull/37526#discussion_r947633106


##########
R/pkg/R/DataFrame.R:
##########
@@ -3577,40 +3577,90 @@ setMethod("str",
 #' This is a no-op if schema doesn't contain column name(s).
 #'
 #' @param x a SparkDataFrame.
-#' @param col a character vector of column names or a Column.
-#' @param ... further arguments to be passed to or from other methods.
-#' @return A SparkDataFrame.
+#' @param col a list of columns or single Column or name.
+#' @param ... additional column(s) if only one column is specified in \code{col}.
+#'            If more than one column is assigned in \code{col}, \code{...}
+#'            should be left empty.
+#' @return A new SparkDataFrame with selected columns.
 #'
 #' @family SparkDataFrame functions
 #' @rdname drop
 #' @name drop
-#' @aliases drop,SparkDataFrame-method
+#' @aliases drop,SparkDataFrame,character-method
+#' @family subsetting functions
 #' @examples
-#'\dontrun{
-#' sparkR.session()
-#' path <- "path/to/file.json"
-#' df <- read.json(path)
-#' drop(df, "col1")
-#' drop(df, c("col1", "col2"))
-#' drop(df, df$col1)
+#' \dontrun{
+#'   drop(df, "*")
+#'   drop(df, "col1", "col2")
+#'   drop(df, df$name, df$age + 1)
+#'   drop(df, c("col1", "col2"))
+#'   drop(df, list(df$name, df$age + 1))
 #' }
-#' @note drop since 2.0.0
-setMethod("drop",
-          signature(x = "SparkDataFrame"),
-          function(x, col) {
-            stopifnot(class(col) == "character" || class(col) == "Column")
+#' @note drop(SparkDataFrame, character) since 2.0.0
+setMethod("drop", signature(x = "SparkDataFrame", col = "character"),

Review Comment:
   Hm, I think you can leverage Union type, see `pkg/R/DataFrame.R:setClassUnion("characterOrColumn", c("character", "Column"))` instead of adding multiple overloaded ones.
   
   And, we can probably don't have to add `list` version



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org