Skip to content

Support optional DuckDB-backed inputs - #289

Draft
stemangiola with Copilot wants to merge 10 commits into
masterfrom
copilot/allow-duckdb-input-with-suggests
Draft

Support optional DuckDB-backed inputs#289
stemangiola with Copilot wants to merge 10 commits into
masterfrom
copilot/allow-duckdb-input-with-suggests

Conversation

Copilot AI commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

sccomp_estimate() could not dispatch on DuckDB-backed lazy tables because no matching S3 method existed. DuckDB should remain optional rather than becoming a hard package dependency.

  • Optional dependencies: Add duckdb and dbplyr to Suggests.
  • S3 dispatch: Register sccomp_estimate.tbl_duckdb_connection().
  • Materialization: Check/install optional packages, collect the lazy table with dplyr::collect(), and delegate to the existing data-frame implementation.
  • Regression coverage: Verify the DuckDB-specific S3 method is registered.
estimate <- sccomp_estimate(
  duckdb_table,
  formula_composition = ~ condition,
  sample = "sample",
  cell_group = "cell_group",
  abundance = "count"
)

Copilot AI and others added 2 commits August 6, 2026 11:27
Co-authored-by: stemangiola <7232890+stemangiola@users.noreply.github.com>
Co-authored-by: stemangiola <7232890+stemangiola@users.noreply.github.com>
Copilot AI changed the title [WIP] Allow DuckDB input without hard dependency Support optional DuckDB-backed inputs Aug 6, 2026
Copilot AI requested a review from stemangiola August 6, 2026 11:28
@stemangiola

Copy link
Copy Markdown
Collaborator

@copilot in unit test, convert the example data into duckdb and run estimate, and make sure the results are the same as with normal df

Copilot AI and others added 5 commits August 6, 2026 11:41
Co-authored-by: stemangiola <7232890+stemangiola@users.noreply.github.com>
Co-authored-by: stemangiola <7232890+stemangiola@users.noreply.github.com>
Co-authored-by: stemangiola <7232890+stemangiola@users.noreply.github.com>
Co-authored-by: stemangiola <7232890+stemangiola@users.noreply.github.com>
Co-authored-by: stemangiola <7232890+stemangiola@users.noreply.github.com>

Copilot AI commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

@copilot in unit test, convert the example data into duckdb and run estimate, and make sure the results are the same as with normal df

Added a DuckDB regression test in commit 8dca580. It writes counts_obj to an in-memory DuckDB table, runs sccomp_estimate() through both paths, and compares the summarized estimates.

Comment thread R/sccomp_estimate.R Outdated
sccomp_estimate.tbl_duckdb_connection <- function(.data, ...) {
check_and_install_packages(c("duckdb", "dbplyr"))

sccomp_estimate.data.frame(dplyr::collect(.data), ...)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should pass the dataset through until you reach the |> count(...) and then you can collect after the count

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated in commit fbe02f8: DuckDB-backed data now remains lazy through the existing sample/cell-group count() checks and is collected afterward, before the remaining data-frame modeling steps.

Co-authored-by: stemangiola <7232890+stemangiola@users.noreply.github.com>
Comment thread R/sccomp_estimate.R Outdated
#' @keywords internal
#' @noRd
make_rectangular_data = function(.data, .sample, .cell_group, .count, formula_composition) {
make_rectangular_data = function(.data, .sample, .cell_group, .count, formula_composition, collect_after_count = FALSE) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of collect_after_count, you can just check is(...) so you avoid creting too many arguments.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied in commit 4bdacca: removed collect_after_count and now detect tbl_duckdb_connection directly in make_rectangular_data() before collecting.

Co-authored-by: stemangiola <7232890+stemangiola@users.noreply.github.com>
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

Successfully merging this pull request may close these issues.

allow duckdb input without putting duckdb in imports but just in suggests, and using check_and_install

2 participants