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.

Sign up for GitHub

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

Jump to bottom

[c++] Propagate Python/R function names to C++ for upgrade/resize methods #3130

Merged
merged 9 commits into from
Oct 4, 2024
Merged

[c++] Propagate Python/R function names to C++ for upgrade/resize methods #3130

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
f852379
_can_set_shape_domainish_helper -> _can_set_shape_domainish_subhelper
johnkerl Oct 3, 2024
2bccafe
can_upgrade_shape take function_name_for_messages argument
johnkerl Oct 3, 2024
0d0814d
_can_set_shape_helper take function_name_for_messages argument
johnkerl Oct 3, 2024
4b6c942
resize take function_name_for_messages argument
johnkerl Oct 4, 2024
e39c8a8
resize_soma_joinid_shape take function_name_for_messages argument
johnkerl Oct 4, 2024
db858d7
upgrade_shape take function_name_for_messages argument
johnkerl Oct 4, 2024
7aada7b
can_resize take function_name_for_messages argument
johnkerl Oct 4, 2024
9315a4f
can_resize_soma_joinid_shape take function_name_for_messages argument
johnkerl Oct 4, 2024
9b54d0a
misc. neatens
johnkerl Oct 4, 2024
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
3 changes: 2 additions & 1 deletion apis/python/src/tiledbsoma/soma_dataframe.cc
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading