fixed tests

Refactor is_range_slice_ungrouped_detected function to accept .data as the first argument for improved clarity. Update corresponding tests to reflect argument order change and ensure correct functionality with various input scenarios.

Author: stemangiola

R/utilities.R

@@ -443,7 +443,7 @@ update_SE_from_tibble <- function(.data_mutated, se, column_belonging = NULL) {
 #' @param .by The .by argument from slice
 #' @return Logical indicating if this is an ungrouped range slice
 #' @noRd
-is_range_slice_ungrouped_detected <- function(slice_args, .data, .by= NULL) {
+is_range_slice_ungrouped_detected <- function(.data, slice_args, .by= NULL) {
   # First check if this is a range slice
   is_range_slice <- any(sapply(slice_args, function(x) {
     if (is.numeric(x)) {
@@ -459,8 +459,12 @@ is_range_slice_ungrouped_detected <- function(slice_args, .data, .by= NULL) {
   }
 
   # If it is a range slice, check if the data is grouped
-  # For SummarizedExperiment objects, only check .by parameter
-  is_grouped <- !quo_is_null(.by)
+  # Check both .by parameter and if the data itself is grouped
+  is_grouped_by_param <- !quo_is_null(.by)
+  is_grouped_by_data <- inherits(.data, "grouped_df") || 
+    (inherits(.data, "SummarizedExperiment") && !is.null(attr(.data, "groups")))
+  
+  is_grouped <- is_grouped_by_param || is_grouped_by_data
 
   # Return TRUE if ungrouped (should throw error)
   !is_grouped
@@ -477,7 +481,7 @@ slice_optimised <- function(.data, ..., .by = NULL, .preserve = FALSE) {
   # In this case, we should return a tibble with the equivalent data
   # Check if this is an ungrouped range slice
   slice_args <- list(...)
-  if (is_range_slice_ungrouped_detected(slice_args, .data, .by)) {
+  if (is_range_slice_ungrouped_detected(.data, slice_args, .by)) {
     # For range slices on ungrouped data, throw an error with a helpful message
     stop("tidySummarizedExperiment says: slice using a range doesn't work on ungrouped data. Please use .by parameter or convert to tibble with as_tibble() before using slice with ranges.")
   }

tests/testthat/test-is_range_slice_detected.R

@@ -1,99 +1,108 @@
 test_that("is_range_slice_ungrouped_detected works correctly", {
   local_edition(3)
 
+  # Load required libraries
+  library(rlang)
+  library(tidySummarizedExperiment)
+  
   # Access the internal function
   is_range_slice_ungrouped_detected <- tidySummarizedExperiment:::is_range_slice_ungrouped_detected
 
   # Load test data
-  library(tidySummarizedExperiment)
   data(pasilla)
 
   # Test with range slice (1:3) on ungrouped data
-  expect_true(is_range_slice_ungrouped_detected(list(1:3), pasilla, NULL))
+  expect_true(is_range_slice_ungrouped_detected(pasilla, list(1:3), quo(NULL)))
 
   # Test with single number on ungrouped data
-  expect_false(is_range_slice_ungrouped_detected(list(1), pasilla, NULL))
-  expect_false(is_range_slice_ungrouped_detected(list(5), pasilla, NULL))
+  expect_false(is_range_slice_ungrouped_detected(pasilla, list(1), quo(NULL)))
+  expect_false(is_range_slice_ungrouped_detected(pasilla, list(5), quo(NULL)))
 
   # Test with multiple single numbers on ungrouped data
-  expect_false(is_range_slice_ungrouped_detected(list(1, 2, 3), pasilla, NULL))
+  expect_false(is_range_slice_ungrouped_detected(pasilla, list(1, 2, 3), quo(NULL)))
 
   # Test with mixed arguments (range and single) on ungrouped data
-  expect_true(is_range_slice_ungrouped_detected(list(1:3, 5), pasilla, NULL))
-  expect_true(is_range_slice_ungrouped_detected(list(1, 2:4), pasilla, NULL))
+  expect_true(is_range_slice_ungrouped_detected(pasilla, list(1:3, 5), quo(NULL)))
+  expect_true(is_range_slice_ungrouped_detected(pasilla, list(1, 2:4), quo(NULL)))
 
   # Test with character arguments on ungrouped data
-  expect_false(is_range_slice_ungrouped_detected(list("a", "b"), pasilla, NULL))
+  expect_false(is_range_slice_ungrouped_detected(pasilla, list("a", "b"), quo(NULL)))
 
   # Test with mixed numeric and character on ungrouped data
-  expect_false(is_range_slice_ungrouped_detected(list(1, "a"), pasilla, NULL))
-  expect_true(is_range_slice_ungrouped_detected(list(1:3, "a"), pasilla, NULL))
+  expect_false(is_range_slice_ungrouped_detected(pasilla, list(1, "a"), quo(NULL)))
+  expect_true(is_range_slice_ungrouped_detected(pasilla, list(1:3, "a"), quo(NULL)))
 
   # Test with empty list on ungrouped data
-  expect_false(is_range_slice_ungrouped_detected(list(), pasilla, NULL))
+  expect_false(is_range_slice_ungrouped_detected(pasilla, list(), quo(NULL)))
 
   # Test with NULL on ungrouped data
-  expect_false(is_range_slice_ungrouped_detected(list(NULL), pasilla, NULL))
+  expect_false(is_range_slice_ungrouped_detected(pasilla, list(quo(NULL)), quo(NULL)))
 
   # Test with logical on ungrouped data
-  expect_false(is_range_slice_ungrouped_detected(list(TRUE, FALSE), pasilla, NULL))
+  expect_false(is_range_slice_ungrouped_detected(pasilla, list(TRUE, FALSE), quo(NULL)))
 
   # Test with complex range on ungrouped data
-  expect_true(is_range_slice_ungrouped_detected(list(seq(1, 10, 2)), pasilla, NULL))
-  expect_true(is_range_slice_ungrouped_detected(list(c(1, 3, 5, 7)), pasilla, NULL))
+  expect_true(is_range_slice_ungrouped_detected(pasilla, list(seq(1, 10, 2)), quo(NULL)))
+  expect_true(is_range_slice_ungrouped_detected(pasilla, list(c(1, 3, 5, 7)), quo(NULL)))
 
   # Test with negative ranges on ungrouped data
-  expect_true(is_range_slice_ungrouped_detected(list(-1:-3), pasilla, NULL))
-  expect_true(is_range_slice_ungrouped_detected(list(1:-1), pasilla, NULL))
+  expect_true(is_range_slice_ungrouped_detected(pasilla, list(-1:-3), quo(NULL)))
+  expect_true(is_range_slice_ungrouped_detected(pasilla, list(1:-1), quo(NULL)))
 
   # Test with decimal ranges on ungrouped data
-  expect_true(is_range_slice_ungrouped_detected(list(seq(1, 3, 0.5)), pasilla, NULL))
+  expect_true(is_range_slice_ungrouped_detected(pasilla, list(seq(1, 3, 0.5)), quo(NULL)))
 })
 
 test_that("is_range_slice_ungrouped_detected handles edge cases", {
   local_edition(3)
 
+  # Load required libraries
+  library(rlang)
+  library(tidySummarizedExperiment)
+  
   # Access the internal function
   is_range_slice_ungrouped_detected <- tidySummarizedExperiment:::is_range_slice_ungrouped_detected
 
   # Load test data
-  library(tidySummarizedExperiment)
   data(pasilla)
 
   # Test with very large ranges on ungrouped data
-  expect_true(is_range_slice_ungrouped_detected(list(1:1000), pasilla, NULL))
+  expect_true(is_range_slice_ungrouped_detected(pasilla, list(1:1000), quo(NULL)))
 
   # Test with single element vector on ungrouped data (should be FALSE)
-  expect_false(is_range_slice_ungrouped_detected(list(c(1)), pasilla, NULL))
+  expect_false(is_range_slice_ungrouped_detected(pasilla, list(c(1)), quo(NULL)))
 
   # Test with named arguments on ungrouped data
-  expect_true(is_range_slice_ungrouped_detected(list(x = 1:3), pasilla, NULL))
-  expect_false(is_range_slice_ungrouped_detected(list(x = 1), pasilla, NULL))
+  expect_true(is_range_slice_ungrouped_detected(pasilla, list(x = 1:3), quo(NULL)))
+  expect_false(is_range_slice_ungrouped_detected(pasilla, list(x = 1), quo(NULL)))
 
   # Test with nested lists on ungrouped data
-  expect_false(is_range_slice_ungrouped_detected(list(list(1:3)), pasilla, NULL))
-  expect_false(is_range_slice_ungrouped_detected(list(list(1)), pasilla, NULL))
+  expect_false(is_range_slice_ungrouped_detected(pasilla, list(list(1:3)), quo(NULL)))
+  expect_false(is_range_slice_ungrouped_detected(pasilla, list(list(1)), quo(NULL)))
 })
 
 test_that("is_range_slice_ungrouped_detected handles grouping correctly", {
   local_edition(3)
 
+  # Load required libraries
+  library(rlang)
+  library(tidySummarizedExperiment)
+  
   # Access the internal function
   is_range_slice_ungrouped_detected <- tidySummarizedExperiment:::is_range_slice_ungrouped_detected
 
   # Load test data
-  library(tidySummarizedExperiment)
   data(pasilla)
 
   # Test ungrouped data with range slice
-  expect_true(is_range_slice_ungrouped_detected(list(1:3), pasilla, NULL))
+  expect_true(is_range_slice_ungrouped_detected(pasilla, list(1:3), quo(NULL)))
 
   # Test grouped data with range slice
   grouped_data <- pasilla |> group_by(.sample)
-  expect_false(is_range_slice_ungrouped_detected(list(1:3), grouped_data, NULL))
+  expect_false(is_range_slice_ungrouped_detected(grouped_data, list(1:3), quo(NULL)))
 
   # Test with .by parameter
-  expect_false(is_range_slice_ungrouped_detected(list(1:3), pasilla, quote(.sample)))
+  expect_false(is_range_slice_ungrouped_detected(pasilla, list(1:3), quo(.sample)))
 })
 
 test_that("slice_optimised handles .by parameter correctly", {
@@ -115,7 +124,7 @@ test_that("slice_optimised handles .by parameter correctly", {
 
   # Test with single slice (should work)
   result <- slice_optimised(pasilla, 1, .by = NULL, .preserve = FALSE)
-  expect_s3_class(result, "SummarizedExperiment")
+  expect_s4_class(result, "SummarizedExperiment")
 })
 
 test_that("slice method works correctly with range detection", {
@@ -133,7 +142,7 @@ test_that("slice method works correctly with range detection", {
 
   # Test single slice on ungrouped data - should work
   result <- pasilla |> dplyr::slice(1)
-  expect_s3_class(result, "SummarizedExperiment")
+  expect_s4_class(result, "SummarizedExperiment")
 
   # Test range slice on grouped data - should work
   result_grouped <- pasilla |> 
@@ -144,5 +153,5 @@ test_that("slice method works correctly with range detection", {
   # Test range slice with .by parameter - should work
   result_by <- pasilla |> 
     dplyr::slice(1:3, .by = .sample)
-  expect_s3_class(result_by, "grouped_df")
+  expect_s4_class(result_by, "SummarizedExperiment")
 })