diff --git a/Makefile b/Makefile index b0a30bd6d5..e8b8e4f061 100644 --- a/Makefile +++ b/Makefile @@ -38,7 +38,10 @@ test: ctest pytest apis/python/tests .PHONY: ctest -ctest: data +ctest: data ctest_update + +.PHONY: ctest_update +ctest_update: ctest --test-dir build/libtiledbsoma -C Release --verbose --rerun-failed --output-on-failure .PHONY: data diff --git a/apis/python/src/tiledbsoma/_dataframe.py b/apis/python/src/tiledbsoma/_dataframe.py index f7ac9b8dc7..821c33a5c6 100644 --- a/apis/python/src/tiledbsoma/_dataframe.py +++ b/apis/python/src/tiledbsoma/_dataframe.py @@ -820,6 +820,9 @@ def _fill_out_slot_soma_domain( ) slot_domain = slot_domain[0], slot_domain[1] elif isinstance(dtype, str): + # Core string dims have no extent and no (core) domain. We return "" here + # simply so we can pass libtiledbsoma "" for domain and extent, while it + # will (and must) ignore these when creating the TileDB schema. slot_domain = "", "" elif np.issubdtype(dtype, NPInteger): iinfo = np.iinfo(cast(NPInteger, dtype)) @@ -887,6 +890,9 @@ def _find_extent_for_domain( if isinstance(dtype, np.dtype) and dtype.itemsize == 1: extent = 1 + # Core string dims have no extent and no (core) domain. We return "" here + # simply so we can pass libtiledbsoma "" for domain and extent, while it + # will (and must) ignore these when creating the TileDB schema. if isinstance(dtype, str): return "" diff --git a/libtiledbsoma/src/soma/soma_array.h b/libtiledbsoma/src/soma/soma_array.h index 4662f92d0c..8d5038aae2 100644 --- a/libtiledbsoma/src/soma/soma_array.h +++ b/libtiledbsoma/src/soma/soma_array.h @@ -726,9 +726,10 @@ class SOMAArray : public SOMAObject { } /** - * Exposed for testing purposes. + * Exposed for testing purposes within this library. + * Not for use by Python/R. */ - CurrentDomain get_current_domain() const { + CurrentDomain get_current_domain_for_test() const { return _get_current_domain(); } @@ -755,9 +756,9 @@ class SOMAArray : public SOMAObject { template std::pair soma_domain_slot(const std::string& name) const { if (has_current_domain()) { - return core_current_domain_slot(name); + return _core_current_domain_slot(name); } else { - return core_domain_slot(name); + return _core_domain_slot(name); } } @@ -771,58 +772,7 @@ class SOMAArray : public SOMAObject { */ template std::pair soma_maxdomain_slot(const std::string& name) const { - return core_domain_slot(name); - } - - /** - * Returns the core current domain at the given dimension. - * - * o For arrays with core current-domain support: - * - soma domain is core current domain - * - soma maxdomain is core domain - * o For arrays without core current-domain support: - * - soma domain is core domain - * - soma maxdomain is core domain - * - core current domain is not accessed at the soma level - * - * @tparam T Domain datatype - * @return Pair of [lower, upper] inclusive bounds. - */ - template - std::pair core_current_domain_slot(const std::string& name) const { - CurrentDomain current_domain = _get_current_domain(); - if (current_domain.is_empty()) { - throw TileDBSOMAError( - "core_current_domain_slot: internal coding error"); - } - if (current_domain.type() != TILEDB_NDRECTANGLE) { - throw TileDBSOMAError( - "core_current_domain_slot: found non-rectangle type"); - } - NDRectangle ndrect = current_domain.ndrectangle(); - - // Convert from two-element array (core API) to pair (tiledbsoma API) - std::array arr = ndrect.range(name); - return std::pair(arr[0], arr[1]); - } - - /** - * Returns the core current domain at the given dimension. - * - * o For arrays with core current-domain support: - * - soma domain is core current domain - * - soma maxdomain is core domain - * o For arrays without core current-domain support: - * - soma domain is core domain - * - soma maxdomain is core domain - * - core current domain is not accessed at the soma level - * - * @tparam T Domain datatype - * @return Pair of [lower, upper] inclusive bounds. - */ - template - std::pair core_domain_slot(const std::string& name) const { - return arr_->schema().domain().dimension(name).domain(); + return _core_domain_slot(name); } /** @@ -945,6 +895,57 @@ class SOMAArray : public SOMAObject { *ctx_->tiledb_ctx(), arr_->schema()); } + /** + * Returns the core current domain at the given dimension. + * + * o For arrays with core current-domain support: + * - soma domain is core current domain + * - soma maxdomain is core domain + * o For arrays without core current-domain support: + * - soma domain is core domain + * - soma maxdomain is core domain + * - core current domain is not accessed at the soma level + * + * @tparam T Domain datatype + * @return Pair of [lower, upper] inclusive bounds. + */ + template + std::pair _core_current_domain_slot(const std::string& name) const { + CurrentDomain current_domain = _get_current_domain(); + if (current_domain.is_empty()) { + throw TileDBSOMAError( + "_core_current_domain_slot: internal coding error"); + } + if (current_domain.type() != TILEDB_NDRECTANGLE) { + throw TileDBSOMAError( + "_core_current_domain_slot: found non-rectangle type"); + } + NDRectangle ndrect = current_domain.ndrectangle(); + + // Convert from two-element array (core API) to pair (tiledbsoma API) + std::array arr = ndrect.range(name); + return std::pair(arr[0], arr[1]); + } + + /** + * Returns the core current domain at the given dimension. + * + * o For arrays with core current-domain support: + * - soma domain is core current domain + * - soma maxdomain is core domain + * o For arrays without core current-domain support: + * - soma domain is core domain + * - soma maxdomain is core domain + * - core current domain is not accessed at the soma level + * + * @tparam T Domain datatype + * @return Pair of [lower, upper] inclusive bounds. + */ + template + std::pair _core_domain_slot(const std::string& name) const { + return arr_->schema().domain().dimension(name).domain(); + } + /** * Helper method for resize and upgrade_shape. */ @@ -963,7 +964,7 @@ class SOMAArray : public SOMAObject { void _check_dims_are_int64(); /** - * With old shape: core domain mapped to tiledbsoma shape; core current + * With old shape: core domain used to map to tiledbsoma shape; core current * domain did not exist. * * With new shape: core domain maps to tiledbsoma maxshape; @@ -1270,6 +1271,16 @@ class SOMAArray : public SOMAObject { uint64_t _nnz_slow(); }; +// These are all specializations to string/bool of various methods +// which require special handling for that type. +// +// Declaring them down here is a bit weird -- they're easy to miss +// on a read-through. However, we're in a bit of a bind regarding +// various compilers: if we do these specializations within the +// `class SOMAArray { ... }`, then one compiler errors if we do +// include `template <>`, while another errors if we don't. +// Doing it down here, no compiler complains. + template <> void SOMAArray::_cast_dictionary_values( ArrowSchema* schema, ArrowArray* array); diff --git a/libtiledbsoma/test/unit_soma_dataframe.cc b/libtiledbsoma/test/unit_soma_dataframe.cc index 1c1388f7a9..45def76e7f 100644 --- a/libtiledbsoma/test/unit_soma_dataframe.cc +++ b/libtiledbsoma/test/unit_soma_dataframe.cc @@ -501,7 +501,8 @@ TEST_CASE_METHOD( // Check current domain auto soma_dataframe = open(OpenMode::read); - CurrentDomain current_domain = soma_dataframe->get_current_domain(); + CurrentDomain current_domain = soma_dataframe + ->get_current_domain_for_test(); if (!use_current_domain) { REQUIRE(current_domain.is_empty()); } else { @@ -602,7 +603,8 @@ TEST_CASE_METHOD( // Check current domain auto soma_dataframe = open(OpenMode::read); - CurrentDomain current_domain = soma_dataframe->get_current_domain(); + CurrentDomain current_domain = soma_dataframe + ->get_current_domain_for_test(); if (!use_current_domain) { REQUIRE(current_domain.is_empty()); } else { @@ -715,7 +717,8 @@ TEST_CASE_METHOD( // Check current domain auto soma_dataframe = open(OpenMode::read); - CurrentDomain current_domain = soma_dataframe->get_current_domain(); + CurrentDomain current_domain = soma_dataframe + ->get_current_domain_for_test(); if (!use_current_domain) { REQUIRE(current_domain.is_empty()); } else { @@ -831,7 +834,8 @@ TEST_CASE_METHOD( // Check current domain auto soma_dataframe = open(OpenMode::read); - CurrentDomain current_domain = soma_dataframe->get_current_domain(); + CurrentDomain current_domain = soma_dataframe + ->get_current_domain_for_test(); if (!use_current_domain) { REQUIRE(current_domain.is_empty()); } else {