Skip to content

Commit

Permalink
[c++] Propagate Python/R function names to C++ for upgrade/resize met…
Browse files Browse the repository at this point in the history 
…hods (#3130)

* _can_set_shape_domainish_helper -> _can_set_shape_domainish_subhelper

* can_upgrade_shape take function_name_for_messages argument

* _can_set_shape_helper take function_name_for_messages argument

* resize take function_name_for_messages argument

* resize_soma_joinid_shape take function_name_for_messages argument

* upgrade_shape take function_name_for_messages argument

* can_resize take function_name_for_messages argument

* can_resize_soma_joinid_shape take function_name_for_messages argument

* misc. neatens
  • Loading branch information
@johnkerl
johnkerl authored Oct 4, 2024
1 parent 47f6b73 commit 26b8929
Show file tree
Hide file tree
Showing 10 changed files with 133 additions and 113 deletions.
3 changes: 2 additions & 1 deletion apis/python/src/tiledbsoma/soma_dataframe.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,8 @@ void load_soma_dataframe(py::module& m) {
"resize_soma_joinid_shape",
[](SOMADataFrame& sdf, int64_t newshape) {
try {
sdf.resize_soma_joinid_shape(newshape);
sdf.resize_soma_joinid_shape(
newshape, "resize_soma_joinid_shape");
} catch (const std::exception& e) {
throw TileDBSOMAError(e.what());
}
Expand Down
4 changes: 2 additions & 2 deletions apis/python/src/tiledbsoma/soma_sparse_ndarray.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ void load_soma_sparse_ndarray(py::module& m) {
"resize",
[](SOMAArray& array, const std::vector<int64_t>& newshape) {
try {
array.resize(newshape);
array.resize(newshape, "resize");
} catch (const std::exception& e) {
throw TileDBSOMAError(e.what());
}
Expand All @@ -132,7 +132,7 @@ void load_soma_sparse_ndarray(py::module& m) {
"tiledbsoma_upgrade_shape",
[](SOMAArray& array, const std::vector<int64_t>& newshape) {
try {
array.upgrade_shape(newshape);
array.upgrade_shape(newshape, "tiledbsoma_upgrade_shape");
} catch (const std::exception& e) {
throw TileDBSOMAError(e.what());
}
Expand Down
2 changes: 1 addition & 1 deletion apis/r/R/RcppExports.R
View file
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ resize <- function(uri, new_shape, ctxxp) {
}

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

tiledbsoma_upgrade_shape <- function(uri, new_shape, ctxxp) {
Expand Down
4 changes: 2 additions & 2 deletions apis/r/src/RcppExports.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,7 @@ 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(SEXP uriSEXP, SEXP new_shapeSEXP, SEXP ctxxpSEXP) {
RcppExport SEXP _tiledbsoma_resize_soma_joinid_shape(SEXP uriSEXP, SEXP new_shapeSEXP, SEXP ctxxpSEXP) {
BEGIN_RCPP
Rcpp::RNGScope rcpp_rngScope_gen;
Rcpp::traits::input_parameter< const std::string& >::type uri(uriSEXP);
Expand Down Expand Up @@ -761,7 +761,7 @@ static const R_CallMethodDef CallEntries[] = {
{"_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", (DL_FUNC) &_tiledbsoma_resize_soma_joinid, 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_c_update_dataframe_schema", (DL_FUNC) &_tiledbsoma_c_update_dataframe_schema, 6},
{"_tiledbsoma_sr_setup", (DL_FUNC) &_tiledbsoma_sr_setup, 10},
Expand Down
6 changes: 3 additions & 3 deletions apis/r/src/rinterface.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ void resize(
// 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);
sr->resize(new_shape_i64, "resize");
sr->close();
}

Expand All @@ -440,7 +440,7 @@ void resize_soma_joinid_shape(
// 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]);
sr->resize_soma_joinid_shape(new_shape_i64[0], "resize_soma_joinid_shape");
sr->close();
}

Expand All @@ -455,7 +455,7 @@ void tiledbsoma_upgrade_shape(
// 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);
sr->upgrade_shape(new_shape_i64, "tiledbsoma_upgrade_shape");
sr->close();
}

Expand Down
77 changes: 43 additions & 34 deletions libtiledbsoma/src/soma/soma_array.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -524,8 +524,8 @@ void SOMAArray::_promote_indexes_to_values(
return _cast_dictionary_values<double>(schema, array);
default:
throw TileDBSOMAError(fmt::format(
"Saw invalid TileDB value type when attempting to "
"promote indexes to values: {}",
"Saw invalid TileDB value type when attempting to promote "
"indexes to values: {}",
tiledb::impl::type_to_str(value_type)));
}
}
Expand Down Expand Up @@ -1452,7 +1452,7 @@ std::pair<bool, std::string> SOMAArray::_can_set_shape_helper(
return std::pair(
false,
fmt::format(
"cannot {}: provided shape has ndim {}, while the array has {}",
"{}: provided shape has ndim {}, while the array has {}",
function_name_for_messages,
arg_ndim,
array_ndim));
Expand Down Expand Up @@ -1481,8 +1481,7 @@ std::pair<bool, std::string> SOMAArray::_can_set_shape_helper(
false,
fmt::format(
"{}: array already has a shape: please use resize rather "
"than "
"tiledbsoma_upgrade_shape.",
"than tiledbsoma_upgrade_shape.",
function_name_for_messages));
}
}
Expand All @@ -1497,7 +1496,7 @@ std::pair<bool, std::string> SOMAArray::_can_set_shape_helper(
//
// if the requested shape fits in the array's core domain, it's good to go
// as a new shape.
auto domain_check = _can_set_shape_domainish_helper(
auto domain_check = _can_set_shape_domainish_subhelper(
newshape, false, function_name_for_messages);
if (!domain_check.first) {
return domain_check;
Expand All @@ -1506,7 +1505,7 @@ std::pair<bool, std::string> SOMAArray::_can_set_shape_helper(
// For new-style arrays, we need to additionally that the the requested
// shape (core current domain) isn't a downsize of the current one.
if (has_shape) {
auto current_domain_check = _can_set_shape_domainish_helper(
auto current_domain_check = _can_set_shape_domainish_subhelper(
newshape, true, function_name_for_messages);
if (!current_domain_check.first) {
return current_domain_check;
Expand All @@ -1519,7 +1518,7 @@ std::pair<bool, std::string> SOMAArray::_can_set_shape_helper(
// This is a helper for _can_set_shape_helper: it's used for comparing
// the user's requested shape against the core current domain or core (max)
// domain.
std::pair<bool, std::string> SOMAArray::_can_set_shape_domainish_helper(
std::pair<bool, std::string> SOMAArray::_can_set_shape_domainish_subhelper(
const std::vector<int64_t>& newshape,
bool check_current_domain,
std::string function_name_for_messages) {
Expand All @@ -1536,8 +1535,7 @@ std::pair<bool, std::string> SOMAArray::_can_set_shape_domainish_helper(
// library-internal code, it's not the user's fault if we got here.
if (dim.type() != TILEDB_INT64) {
throw TileDBSOMAError(fmt::format(
"{}: internal error: expected {} dim to "
"be {}; got {}",
"{}: internal error: expected {} dim to be {}; got {}",
function_name_for_messages,
dim_name,
tiledb::impl::type_to_str(TILEDB_INT64),
Expand All @@ -1553,7 +1551,7 @@ std::pair<bool, std::string> SOMAArray::_can_set_shape_domainish_helper(
return std::pair(
false,
fmt::format(
"cannot {} for {}: new {} < existing shape {}",
"{} for {}: new {} < existing shape {}",
function_name_for_messages,
dim_name,
newshape[i],
Expand All @@ -1569,7 +1567,7 @@ std::pair<bool, std::string> SOMAArray::_can_set_shape_domainish_helper(
return std::pair(
false,
fmt::format(
"cannot {} for {}: new {} < maxshape {}",
"{} for {}: new {} < maxshape {}",
function_name_for_messages,
dim_name,
newshape[i],
Expand All @@ -1580,15 +1578,17 @@ std::pair<bool, std::string> SOMAArray::_can_set_shape_domainish_helper(
return std::pair(true, "");
}

std::pair<bool, std::string> SOMAArray::can_resize_soma_joinid(
int64_t newshape) {
std::pair<bool, std::string> SOMAArray::can_resize_soma_joinid_shape(
int64_t newshape, std::string function_name_for_messages) {
// Fail if the array doesn't already have a shape yet (they should upgrade
// first).
if (!has_current_domain()) {
return std::pair(
false,
"can_resize_soma_joinid: dataframe currently has no domain set: "
"please use tiledbsoma_upgrade_domain.");
fmt::format(
"{}: dataframe currently has no domain set: please "
"upgrade the array.",
function_name_for_messages));
}

// OK if soma_joinid isn't a dim.
Expand All @@ -1602,8 +1602,8 @@ std::pair<bool, std::string> SOMAArray::can_resize_soma_joinid(
return std::pair(
false,
fmt::format(
"cannot resize_soma_joinid_shape: new soma_joinid shape {} < "
"existing shape {}",
"{}: new soma_joinid shape {} < existing shape {}",
function_name_for_messages,
newshape,
cur_dom_lo_hi.second));
}
Expand All @@ -1614,8 +1614,8 @@ std::pair<bool, std::string> SOMAArray::can_resize_soma_joinid(
return std::pair(
false,
fmt::format(
"cannot resize_soma_joinid_shape: new soma_joinid shape {} > "
"maxshape {}",
"{}: new soma_joinid shape {} > maxshape {}",
function_name_for_messages,
newshape,
dom_lo_hi.second));
}
Expand All @@ -1624,27 +1624,34 @@ std::pair<bool, std::string> SOMAArray::can_resize_soma_joinid(
return std::pair(true, "");
}

void SOMAArray::resize(const std::vector<int64_t>& newshape) {
void SOMAArray::resize(
const std::vector<int64_t>& newshape,
std::string function_name_for_messages) {
if (_get_current_domain().is_empty()) {
throw TileDBSOMAError(
"[SOMAArray::resize] array must already have a shape");
throw TileDBSOMAError(fmt::format(
"{} array must already have a shape", function_name_for_messages));
}
_set_current_domain_from_shape(newshape);
_set_current_domain_from_shape(newshape, function_name_for_messages);
}

void SOMAArray::upgrade_shape(const std::vector<int64_t>& newshape) {
void SOMAArray::upgrade_shape(
const std::vector<int64_t>& newshape,
std::string function_name_for_messages) {
if (!_get_current_domain().is_empty()) {
throw TileDBSOMAError(
"[SOMAArray::resize] array must not already have a shape");
throw TileDBSOMAError(fmt::format(
"{}: array must not already have a shape",
function_name_for_messages));
}
_set_current_domain_from_shape(newshape);
_set_current_domain_from_shape(newshape, function_name_for_messages);
}

void SOMAArray::_set_current_domain_from_shape(
const std::vector<int64_t>& newshape) {
const std::vector<int64_t>& newshape,
std::string function_name_for_messages) {
if (mq_->query_type() != TILEDB_WRITE) {
throw TileDBSOMAError(
"[SOMAArray::resize] array must be opened in write mode");
throw TileDBSOMAError(fmt::format(
"{} array must be opened in write mode",
function_name_for_messages));
}

// Variant-indexed dataframes must use a separate path
Expand Down Expand Up @@ -1677,10 +1684,12 @@ void SOMAArray::_set_current_domain_from_shape(
schema_evolution.array_evolve(uri_);
}

void SOMAArray::resize_soma_joinid_shape(int64_t newshape) {
void SOMAArray::resize_soma_joinid_shape(
int64_t newshape, std::string function_name_for_messages) {
if (mq_->query_type() != TILEDB_WRITE) {
throw TileDBSOMAError(
"[SOMAArray::resize] array must be opened in write mode");
throw TileDBSOMAError(fmt::format(
"{}: array must be opened in write mode",
function_name_for_messages));
}

ArraySchema schema = arr_->schema();
Expand Down
34 changes: 22 additions & 12 deletions libtiledbsoma/src/soma/soma_array.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -997,8 +997,7 @@ class SOMAArray : public SOMAObject {
default:
throw std::runtime_error(
"internal coding error in "
"SOMAArray::_core_domainish_slot_string: "
"unknown kind");
"SOMAArray::_core_domainish_slot_string: unknown kind");
}
}

Expand Down Expand Up @@ -1061,8 +1060,10 @@ class SOMAArray : public SOMAObject {
* existing core domain.
*/
std::pair<bool, std::string> can_resize(
const std::vector<int64_t>& newshape) {
return _can_set_shape_helper(newshape, true, "resize");
const std::vector<int64_t>& newshape,
std::string function_name_for_messages) {
return _can_set_shape_helper(
newshape, true, function_name_for_messages);
}

/**
Expand All @@ -1083,16 +1084,18 @@ class SOMAArray : public SOMAObject {
* domain.
*/
std::pair<bool, std::string> can_upgrade_shape(
const std::vector<int64_t>& newshape) {
const std::vector<int64_t>& newshape,
std::string function_name_for_messages) {
return _can_set_shape_helper(
newshape, false, "tiledbsoma_upgrade_shape");
newshape, false, function_name_for_messages);
}

/**
* This is similar to can_upgrade_shape, but it's a can-we call
* for maybe_resize_soma_joinid.
*/
std::pair<bool, std::string> can_resize_soma_joinid(int64_t newshape);
std::pair<bool, std::string> can_resize_soma_joinid_shape(
int64_t newshape, std::string function_name_for_messages);

/**
* @brief Resize the shape (what core calls "current domain") up to the
Expand All @@ -1105,15 +1108,19 @@ class SOMAArray : public SOMAObject {
* @return Nothing. Raises an exception if the resize would be a downsize,
* which is not supported.
*/
void resize(const std::vector<int64_t>& newshape);
void resize(
const std::vector<int64_t>& newshape,
std::string function_name_for_messages);

/**
* @brief Given an old-style array without current domain, sets its
* current domain. This is applicable only to arrays having all dims
* of int64 type. Namely, all SparseNDArray/DenseNDArray, and
* default-indexed DataFrame.
*/
void upgrade_shape(const std::vector<int64_t>& newshape);
void upgrade_shape(
const std::vector<int64_t>& newshape,
std::string function_name_for_messages);

/**
* @brief Increases the tiledbsoma shape up to at most the maxshape,
Expand All @@ -1129,7 +1136,8 @@ class SOMAArray : public SOMAObject {
* @return Throws if the requested shape exceeds the array's create-time
* maxshape. Throws if the array does not have current-domain support.
*/
void resize_soma_joinid_shape(int64_t newshape);
void resize_soma_joinid_shape(
int64_t newshape, std::string function_name_for_messages);

protected:
// These two are for use nominally by SOMADataFrame. This could be moved in
Expand Down Expand Up @@ -1198,15 +1206,17 @@ class SOMAArray : public SOMAObject {
/**
* This is a second-level code-dedupe helper for _can_set_shape_helper.
*/
std::pair<bool, std::string> _can_set_shape_domainish_helper(
std::pair<bool, std::string> _can_set_shape_domainish_subhelper(
const std::vector<int64_t>& newshape,
bool check_current_domain,
std::string function_name_for_messages);

/**
* This is a code-dedupe helper method for resize and upgrade_shape.
*/
void _set_current_domain_from_shape(const std::vector<int64_t>& newshape);
void _set_current_domain_from_shape(
const std::vector<int64_t>& newshape,
std::string function_name_for_messages);

/**
* While SparseNDArray, DenseNDArray, and default-indexed DataFrame
Expand Down
Loading

0 comments on commit 26b8929

Please sign in to comment.