Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[r] Proper prefixing for shape-related methods #3237

Merged
merged 3 commits into from
Oct 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apis/r/DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ Description: Interface for working with 'TileDB'-based Stack of Matrices,
like those commonly used for single cell data analysis. It is documented at
<https://github.com/single-cell-data>; a formal specification available is at
<https://github.com/single-cell-data/SOMA/blob/main/abstract_specification.md>.
Version: 1.15.99.9
Version: 1.15.99.10
Authors@R: c(
person(given = "Aaron", family = "Wolen",
role = c("cre", "aut"), email = "[email protected]",
Expand Down
1 change: 1 addition & 0 deletions apis/r/NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
* Fix `is_named_list` bug for half-named lists [#3183](https://github.com/single-cell-data/TileDB-SOMA/pull/3183)
* Expose block/random writer for sparse arrays [#3204](https://github.com/single-cell-data/TileDB-SOMA/pull/3204)
* Min-sizing for dataframes/arrays with new shape feature [#3208](https://github.com/single-cell-data/TileDB-SOMA/pull/3208)
* Proper prefixing for shape-related methods [#3237](https://github.com/single-cell-data/TileDB-SOMA/pull/3237)

# tiledbsoma 1.14.1

Expand Down
12 changes: 6 additions & 6 deletions apis/r/R/RcppExports.R
Original file line number Diff line number Diff line change
Expand Up @@ -238,16 +238,16 @@ c_schema <- function(uri, ctxxp) {
.Call(`_tiledbsoma_c_schema`, uri, ctxxp)
}

resize <- function(uri, new_shape, ctxxp) {
invisible(.Call(`_tiledbsoma_resize`, uri, new_shape, ctxxp))
resize <- function(uri, new_shape, function_name_for_messages, ctxxp) {
invisible(.Call(`_tiledbsoma_resize`, uri, new_shape, function_name_for_messages, ctxxp))
}

resize_soma_joinid_shape <- function(uri, new_shape, ctxxp) {
invisible(.Call(`_tiledbsoma_resize_soma_joinid_shape`, uri, new_shape, ctxxp))
resize_soma_joinid_shape <- function(uri, new_shape, function_name_for_messages, ctxxp) {
invisible(.Call(`_tiledbsoma_resize_soma_joinid_shape`, uri, new_shape, function_name_for_messages, ctxxp))
}

tiledbsoma_upgrade_shape <- function(uri, new_shape, ctxxp) {
invisible(.Call(`_tiledbsoma_tiledbsoma_upgrade_shape`, uri, new_shape, ctxxp))
tiledbsoma_upgrade_shape <- function(uri, new_shape, function_name_for_messages, ctxxp) {
invisible(.Call(`_tiledbsoma_tiledbsoma_upgrade_shape`, uri, new_shape, function_name_for_messages, ctxxp))
}

c_update_dataframe_schema <- function(uri, ctxxp, column_names_to_drop, add_cols_types, add_cols_enum_value_types, add_cols_enum_ordered) {
Expand Down
7 changes: 4 additions & 3 deletions apis/r/R/SOMADataFrame.R
Original file line number Diff line number Diff line change
Expand Up @@ -415,13 +415,14 @@ SOMADataFrame <- R6::R6Class(
#' @param new_shape An integer, greater than or equal to 1 + the
#' `soma_joinid` domain slot.
#' @return No return value
resize_soma_joinid_shape = function(new_shape) {

tiledbsoma_resize_soma_joinid_shape = function(new_shape) {
stopifnot("'new_shape' must be an integer" = rlang::is_integerish(new_shape, n = 1) ||
(bit64::is.integer64(new_shape) && length(new_shape) == 1)
)
# Checking slotwise new shape >= old shape, and <= max_shape, is already done in libtiledbsoma
invisible(resize_soma_joinid_shape(self$uri, new_shape, private$.soma_context))
invisible(
resize_soma_joinid_shape(
self$uri, new_shape, .name_of_function(), private$.soma_context))
}

),
Expand Down
4 changes: 2 additions & 2 deletions apis/r/R/SOMANDArrayBase.R
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ SOMANDArrayBase <- R6::R6Class(
(bit64::is.integer64(new_shape) && length(new_shape) == self$ndim())
)
# Checking slotwise new shape >= old shape, and <= max_shape, is already done in libtiledbsoma
resize(self$uri, new_shape, private$.soma_context)
resize(self$uri, new_shape, .name_of_function(), private$.soma_context)
},

#' @description Allows the array to have a resizeable shape as described in the
Expand All @@ -125,7 +125,7 @@ SOMANDArrayBase <- R6::R6Class(
(bit64::is.integer64(shape) && length(shape) == self$ndim())
)
# Checking slotwise new shape >= old shape, and <= max_shape, is already done in libtiledbsoma
tiledbsoma_upgrade_shape(self$uri, shape, private$.soma_context)
tiledbsoma_upgrade_shape(self$uri, shape, .name_of_function(), private$.soma_context)
}

),
Expand Down
19 changes: 19 additions & 0 deletions apis/r/R/utils.R
Original file line number Diff line number Diff line change
Expand Up @@ -303,3 +303,22 @@ SOMA_ENCODING_VERSION <- "1.1.0"
##' @importFrom spdl setup
##' @useDynLib tiledbsoma, .registration=TRUE
NULL

# This is for internal logging purposes. Context:
# * We have R (and Python) code with function names the user invokes.
# * These call C++ functions which can throw their own error messages.
# * It's crucial that the C++ code "knows" the name of the function
# as typed by the user, not whatever (possibly out-of-date) guess
# the C++ code may have.
.name_of_function <- function() {
# Tricky bits:
# * This might be `obj$foo`
# * The sys.call can return a parse-tree component (typeof = language)
# with the '$', 'obj', and 'foo' -- hence the as.character
# * Even then there can be a second component returned like 'c(1)'
# -- hence the [[1]]
# * Then remove the 'obj$' from 'obj$foo'
name <- as.character(sys.call(sys.parent(n=1)))[[1]]
name <- sub('.*\\$', replacement = '', x = name)
return(name)
}
2 changes: 1 addition & 1 deletion apis/r/R/write_soma.R
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ write_soma.data.frame <- function(
}
}
if (ingest_mode %in% c('resume') && sdf$tiledbsoma_has_upgraded_domain()) {
sdf$resize_soma_joinid_shape(nrow(x))
sdf$tiledbsoma_resize_soma_joinid_shape(nrow(x))
}
if (!is.null(tbl)) {
sdf$write(tbl)
Expand Down
27 changes: 15 additions & 12 deletions apis/r/src/RcppExports.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -553,38 +553,41 @@ BEGIN_RCPP
END_RCPP
}
// resize
void resize(const std::string& uri, Rcpp::NumericVector new_shape, Rcpp::XPtr<somactx_wrap_t> ctxxp);
RcppExport SEXP _tiledbsoma_resize(SEXP uriSEXP, SEXP new_shapeSEXP, SEXP ctxxpSEXP) {
void resize(const std::string& uri, Rcpp::NumericVector new_shape, std::string function_name_for_messages, Rcpp::XPtr<somactx_wrap_t> ctxxp);
RcppExport SEXP _tiledbsoma_resize(SEXP uriSEXP, SEXP new_shapeSEXP, SEXP function_name_for_messagesSEXP, SEXP ctxxpSEXP) {
BEGIN_RCPP
Rcpp::RNGScope rcpp_rngScope_gen;
Rcpp::traits::input_parameter< const std::string& >::type uri(uriSEXP);
Rcpp::traits::input_parameter< Rcpp::NumericVector >::type new_shape(new_shapeSEXP);
Rcpp::traits::input_parameter< std::string >::type function_name_for_messages(function_name_for_messagesSEXP);
Rcpp::traits::input_parameter< Rcpp::XPtr<somactx_wrap_t> >::type ctxxp(ctxxpSEXP);
resize(uri, new_shape, ctxxp);
resize(uri, new_shape, function_name_for_messages, ctxxp);
return R_NilValue;
END_RCPP
}
// resize_soma_joinid_shape
void resize_soma_joinid_shape(const std::string& uri, Rcpp::NumericVector new_shape, Rcpp::XPtr<somactx_wrap_t> ctxxp);
RcppExport SEXP _tiledbsoma_resize_soma_joinid_shape(SEXP uriSEXP, SEXP new_shapeSEXP, SEXP ctxxpSEXP) {
void resize_soma_joinid_shape(const std::string& uri, Rcpp::NumericVector new_shape, std::string function_name_for_messages, Rcpp::XPtr<somactx_wrap_t> ctxxp);
RcppExport SEXP _tiledbsoma_resize_soma_joinid_shape(SEXP uriSEXP, SEXP new_shapeSEXP, SEXP function_name_for_messagesSEXP, SEXP ctxxpSEXP) {
BEGIN_RCPP
Rcpp::RNGScope rcpp_rngScope_gen;
Rcpp::traits::input_parameter< const std::string& >::type uri(uriSEXP);
Rcpp::traits::input_parameter< Rcpp::NumericVector >::type new_shape(new_shapeSEXP);
Rcpp::traits::input_parameter< std::string >::type function_name_for_messages(function_name_for_messagesSEXP);
Rcpp::traits::input_parameter< Rcpp::XPtr<somactx_wrap_t> >::type ctxxp(ctxxpSEXP);
resize_soma_joinid_shape(uri, new_shape, ctxxp);
resize_soma_joinid_shape(uri, new_shape, function_name_for_messages, ctxxp);
return R_NilValue;
END_RCPP
}
// tiledbsoma_upgrade_shape
void tiledbsoma_upgrade_shape(const std::string& uri, Rcpp::NumericVector new_shape, Rcpp::XPtr<somactx_wrap_t> ctxxp);
RcppExport SEXP _tiledbsoma_tiledbsoma_upgrade_shape(SEXP uriSEXP, SEXP new_shapeSEXP, SEXP ctxxpSEXP) {
void tiledbsoma_upgrade_shape(const std::string& uri, Rcpp::NumericVector new_shape, std::string function_name_for_messages, Rcpp::XPtr<somactx_wrap_t> ctxxp);
RcppExport SEXP _tiledbsoma_tiledbsoma_upgrade_shape(SEXP uriSEXP, SEXP new_shapeSEXP, SEXP function_name_for_messagesSEXP, SEXP ctxxpSEXP) {
BEGIN_RCPP
Rcpp::RNGScope rcpp_rngScope_gen;
Rcpp::traits::input_parameter< const std::string& >::type uri(uriSEXP);
Rcpp::traits::input_parameter< Rcpp::NumericVector >::type new_shape(new_shapeSEXP);
Rcpp::traits::input_parameter< std::string >::type function_name_for_messages(function_name_for_messagesSEXP);
Rcpp::traits::input_parameter< Rcpp::XPtr<somactx_wrap_t> >::type ctxxp(ctxxpSEXP);
tiledbsoma_upgrade_shape(uri, new_shape, ctxxp);
tiledbsoma_upgrade_shape(uri, new_shape, function_name_for_messages, ctxxp);
return R_NilValue;
END_RCPP
}
Expand Down Expand Up @@ -816,9 +819,9 @@ static const R_CallMethodDef CallEntries[] = {
{"_tiledbsoma_c_dimnames", (DL_FUNC) &_tiledbsoma_c_dimnames, 2},
{"_tiledbsoma_c_attrnames", (DL_FUNC) &_tiledbsoma_c_attrnames, 2},
{"_tiledbsoma_c_schema", (DL_FUNC) &_tiledbsoma_c_schema, 2},
{"_tiledbsoma_resize", (DL_FUNC) &_tiledbsoma_resize, 3},
{"_tiledbsoma_resize_soma_joinid_shape", (DL_FUNC) &_tiledbsoma_resize_soma_joinid_shape, 3},
{"_tiledbsoma_tiledbsoma_upgrade_shape", (DL_FUNC) &_tiledbsoma_tiledbsoma_upgrade_shape, 3},
{"_tiledbsoma_resize", (DL_FUNC) &_tiledbsoma_resize, 4},
{"_tiledbsoma_resize_soma_joinid_shape", (DL_FUNC) &_tiledbsoma_resize_soma_joinid_shape, 4},
{"_tiledbsoma_tiledbsoma_upgrade_shape", (DL_FUNC) &_tiledbsoma_tiledbsoma_upgrade_shape, 4},
{"_tiledbsoma_c_update_dataframe_schema", (DL_FUNC) &_tiledbsoma_c_update_dataframe_schema, 6},
{"_tiledbsoma_sr_setup", (DL_FUNC) &_tiledbsoma_sr_setup, 10},
{"_tiledbsoma_sr_complete", (DL_FUNC) &_tiledbsoma_sr_complete, 1},
Expand Down
9 changes: 6 additions & 3 deletions apis/r/src/rinterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -421,41 +421,44 @@ SEXP c_schema(const std::string& uri, Rcpp::XPtr<somactx_wrap_t> ctxxp) {
void resize(
const std::string& uri,
Rcpp::NumericVector new_shape,
std::string function_name_for_messages,
Rcpp::XPtr<somactx_wrap_t> ctxxp) {
// This function is solely for SparseNDArray and DenseNDArray for which the
// dims are required by the SOMA spec to be of type int64. Domain-resize for
// variant-indexed dataframes will be separate work as tracked on
// https://github.com/single-cell-data/TileDB-SOMA/issues/2407.
auto sr = tdbs::SOMAArray::open(OpenMode::write, uri, ctxxp->ctxptr);
std::vector<int64_t> new_shape_i64 = i64_from_rcpp_numeric(new_shape);
sr->resize(new_shape_i64, "resize");
sr->resize(new_shape_i64, function_name_for_messages);
sr->close();
}

// [[Rcpp::export]]
void resize_soma_joinid_shape(
const std::string& uri,
Rcpp::NumericVector new_shape,
std::string function_name_for_messages,
Rcpp::XPtr<somactx_wrap_t> ctxxp) {
// This function is solely for SOMADataFrame.
auto sr = tdbs::SOMADataFrame::open(uri, OpenMode::write, ctxxp->ctxptr);
std::vector<int64_t> new_shape_i64 = i64_from_rcpp_numeric(new_shape);
sr->resize_soma_joinid_shape(new_shape_i64[0], "resize_soma_joinid_shape");
sr->resize_soma_joinid_shape(new_shape_i64[0], function_name_for_messages);
sr->close();
}

// [[Rcpp::export]]
void tiledbsoma_upgrade_shape(
const std::string& uri,
Rcpp::NumericVector new_shape,
std::string function_name_for_messages,
Rcpp::XPtr<somactx_wrap_t> ctxxp) {
// This function is solely for SparseNDArray and DenseNDArray for which the
// dims are required by the SOMA spec to be of type int64. Domain-resize for
// variant-indexed dataframes will be separate work as tracked on
// https://github.com/single-cell-data/TileDB-SOMA/issues/2407.
auto sr = tdbs::SOMAArray::open(OpenMode::write, uri, ctxxp->ctxptr);
std::vector<int64_t> new_shape_i64 = i64_from_rcpp_numeric(new_shape);
sr->upgrade_shape(new_shape_i64, "tiledbsoma_upgrade_shape");
sr->upgrade_shape(new_shape_i64, function_name_for_messages);
sr->close();
}

Expand Down
8 changes: 4 additions & 4 deletions apis/r/tests/testthat/test-shape.R
Original file line number Diff line number Diff line change
Expand Up @@ -199,11 +199,11 @@ test_that("SOMADataFrame shape", {
sdf <- SOMADataFrameOpen(uri, "WRITE")
if (has_soma_joinid_dim) {
# It's an error to downsize
expect_error(sdf$resize_soma_joinid_shape(new_shape))
expect_error(sdf$tiledbsoma_resize_soma_joinid_shape(new_shape))
} else {
# There is no problem when soma_joinid is not a dim --
# sdf$resize_soma_joinid_shape is a no-op in that case
expect_no_condition(sdf$resize_soma_joinid_shape(new_shape))
# sdf$tiledbsoma_resize_soma_joinid_shape is a no-op in that case
expect_no_condition(sdf$tiledbsoma_resize_soma_joinid_shape(new_shape))
}
sdf$close()

Expand Down Expand Up @@ -238,7 +238,7 @@ test_that("SOMADataFrame shape", {

# Test resize
sdf <- SOMADataFrameOpen(uri, "WRITE")
sdf$resize_soma_joinid_shape(new_shape)
sdf$tiledbsoma_resize_soma_joinid_shape(new_shape)
sdf$close();

# Test writes out of old bounds, within new bounds, after resize
Expand Down
2 changes: 1 addition & 1 deletion apis/r/tests/testthat/test-write-soma-resume.R
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ test_that("Resume-mode data frames", {

if (.new_shape_feature_flag_is_enabled()) {
sdfp$reopen("WRITE")
sdfp$resize_soma_joinid_shape(nrow(co2))
sdfp$tiledbsoma_resize_soma_joinid_shape(nrow(co2))
}

expect_s3_class(
Expand Down
4 changes: 2 additions & 2 deletions libtiledbsoma/src/soma/soma_array.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1665,14 +1665,14 @@ void SOMAArray::_set_shape_helper(
// Upgrading an array to install a current domain
if (!_get_current_domain().is_empty()) {
throw TileDBSOMAError(fmt::format(
"{}: array must not already have a shape",
"{}: array must not already have a shape: please upgrade it",
function_name_for_messages));
}
} else {
// Expanding an array's current domain
if (_get_current_domain().is_empty()) {
throw TileDBSOMAError(fmt::format(
"{} array must already have a shape",
"{} array must already have a shape: please upgrade it",
function_name_for_messages));
}
}
Expand Down