Skip to content

Commit

Permalink
Fix Windows build regressions and stop using deprecated TileDB APIs. (#…
Browse files Browse the repository at this point in the history
…2445)

* Fix Windows build regressions.

* Fix experiment and measurement tests to not call the base `open` method.

* Stop using deprecated TileDB APIs in tests.
  • Loading branch information
teo-tsirpanis authored Apr 17, 2024
1 parent 8c07c6f commit d4138cc
Show file tree
Hide file tree
Showing 13 changed files with 36 additions and 30 deletions.
9 changes: 4 additions & 5 deletions libtiledbsoma/src/reindexer/reindexer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@

#include "reindexer.h"
#include <thread_pool/thread_pool.h>
#include <unistd.h>
#include <thread>
#include "khash.h"
#include "soma/enums.h"
Expand Down Expand Up @@ -78,14 +77,14 @@ void IntIndexer::map_locations(const int64_t* keys, size_t size) {
fmt::format("[Re-indexer] Thread pool started and hash table created"));
}

void IntIndexer::lookup(const int64_t* keys, int64_t* results, int size) {
void IntIndexer::lookup(const int64_t* keys, int64_t* results, size_t size) {
if (size == 0) {
return;
}
// Single thread checks
if (context_ == nullptr || context_->thread_pool() == nullptr ||
context_->thread_pool()->concurrency_level() == 1) {
for (int i = 0; i < size; i++) {
for (size_t i = 0; i < size; i++) {
auto k = kh_get(m64, hash_, keys[i]);
if (k == kh_end(hash_)) {
// According to pandas behavior
Expand All @@ -109,10 +108,10 @@ void IntIndexer::lookup(const int64_t* keys, int64_t* results, int size) {
thread_chunk_size = 1;
}

for (size_t i = 0; i < size_t(size); i += thread_chunk_size) {
for (size_t i = 0; i < size; i += thread_chunk_size) {
size_t start = i;
size_t end = i + thread_chunk_size;
if (end > size_t(size)) {
if (end > size) {
end = size;
}
LOG_DEBUG(fmt::format(
Expand Down
5 changes: 2 additions & 3 deletions libtiledbsoma/src/reindexer/reindexer.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
#define TILEDBSOMA_REINDEXER_H

#include <assert.h>
#include <unistd.h>
#include <memory>
#include <stdexcept>
#include <vector>
Expand Down Expand Up @@ -64,7 +63,7 @@ class IntIndexer {
* @param size // Number of key array
* @return and array of looked up value (same size as keys)
*/
void lookup(const int64_t* keys, int64_t* results, int size);
void lookup(const int64_t* keys, int64_t* results, size_t size);
void lookup(
const std::vector<int64_t>& keys, std::vector<int64_t>& results) {
if (keys.size() != results.size())
Expand All @@ -89,7 +88,7 @@ class IntIndexer {
/*
* Number of elements in the map set by map_locations
*/
int map_size_ = 0;
size_t map_size_ = 0;
};

} // namespace tiledbsoma
Expand Down
2 changes: 1 addition & 1 deletion libtiledbsoma/src/soma/soma_collection.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ class SOMACollection : public SOMAGroup {
mode,
uri,
ctx,
std::string(std::filesystem::path(uri).filename()), // group name
std::filesystem::path(uri).filename().string(), // group name
timestamp){};

SOMACollection(const SOMAGroup& other)
Expand Down
2 changes: 1 addition & 1 deletion libtiledbsoma/src/soma/soma_dataframe.h
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ class SOMADataFrame : public SOMAArray {
mode,
uri,
ctx,
std::string(std::filesystem::path(uri).filename()), // array name
std::filesystem::path(uri).filename().string(), // array name
column_names,
"auto", // batch_size
result_order,
Expand Down
2 changes: 1 addition & 1 deletion libtiledbsoma/src/soma/soma_dense_ndarray.h
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ class SOMADenseNDArray : public SOMAArray {
mode,
uri,
ctx,
std::string(std::filesystem::path(uri).filename()), // array name
std::filesystem::path(uri).filename().string(), // array name
column_names,
"auto", // batch_size
result_order,
Expand Down
2 changes: 0 additions & 2 deletions libtiledbsoma/src/soma/soma_experiment.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,6 @@ class SOMAExperiment : public SOMACollection {
SOMAExperiment(SOMAExperiment&&) = default;
~SOMAExperiment() = default;

using SOMACollection::open;

private:
//===================================================================
//= private non-static
Expand Down
2 changes: 0 additions & 2 deletions libtiledbsoma/src/soma/soma_measurement.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,6 @@ class SOMAMeasurement : public SOMACollection {
SOMAMeasurement(SOMAMeasurement&&) = default;
~SOMAMeasurement() = default;

using SOMACollection::open;

private:
//===================================================================
//= private non-static
Expand Down
2 changes: 1 addition & 1 deletion libtiledbsoma/src/soma/soma_sparse_ndarray.h
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ class SOMASparseNDArray : public SOMAArray {
mode,
uri,
ctx,
std::string(std::filesystem::path(uri).filename()), // array name
std::filesystem::path(uri).filename().string(), // array name
column_names,
"auto", // batch_size
result_order,
Expand Down
6 changes: 3 additions & 3 deletions libtiledbsoma/src/utils/arrow_adapter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ std::unique_ptr<ArrowSchema> ArrowAdapter::arrow_schema_from_tiledb_array(
auto dict = (ArrowSchema*)malloc(sizeof(ArrowSchema));
dict->format = strdup(
ArrowAdapter::to_arrow_format(enmr.type(), false).data());
if (enmr.type() == TILEDB_STRING_ASCII or
if (enmr.type() == TILEDB_STRING_ASCII ||
enmr.type() == TILEDB_CHAR) {
dict->format = strdup("z");
} else {
Expand Down Expand Up @@ -583,8 +583,8 @@ ArrowAdapter::to_arrow(std::shared_ptr<ColumnBuffer> column) {
// future refactor as ColumnBuffer::get_enumeration_info
// returns std::optional where std::nullopt indicates the
// column does not contain enumerated values.
if (enmr->type() == TILEDB_STRING_ASCII or
enmr->type() == TILEDB_STRING_UTF8 or enmr->type() == TILEDB_CHAR) {
if (enmr->type() == TILEDB_STRING_ASCII ||
enmr->type() == TILEDB_STRING_UTF8 || enmr->type() == TILEDB_CHAR) {
auto dict_vec = enmr->as_vector<std::string>();
column->convert_enumeration();
dict_arr->buffers[1] = column->enum_offsets().data();
Expand Down
3 changes: 2 additions & 1 deletion libtiledbsoma/test/test_indexer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ KHASH_MAP_INIT_INT64(m64, int64_t)
// exception.
std::vector<bool> uniqueness = {false, false, true, true, true};

bool run_test(int id, std::vector<int64_t> keys, std::vector<int64_t> lookups) {
bool run_test(
size_t id, std::vector<int64_t> keys, std::vector<int64_t> lookups) {
try {
std::vector<int64_t> indexer_results;
indexer_results.resize(lookups.size());
Expand Down
5 changes: 4 additions & 1 deletion libtiledbsoma/test/unit_soma_array.cc
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,10 @@ std::tuple<std::vector<int64_t>, std::vector<int>> write_array(

// Read from TileDB Array to get expected data
Array tiledb_array(
*ctx->tiledb_ctx(), uri, TILEDB_READ, timestamp + num_fragments - 1);
*ctx->tiledb_ctx(),
uri,
TILEDB_READ,
TemporalPolicy(TimeTravel, timestamp + num_fragments - 1));
tiledb_array.reopen();

std::vector<int64_t> expected_d0(num_cells_per_fragment * num_fragments);
Expand Down
24 changes: 16 additions & 8 deletions libtiledbsoma/test/unit_soma_collection.cc
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,8 @@ TEST_CASE("SOMAExperiment: metadata") {
soma_experiment->close();

// Read metadata
soma_experiment->open(OpenMode::read, TimestampRange(0, 2));
soma_experiment = SOMAExperiment::open(
uri, OpenMode::read, ctx, TimestampRange(0, 2));
REQUIRE(soma_experiment->metadata_num() == 3);
REQUIRE(soma_experiment->has_metadata("soma_object_type"));
REQUIRE(soma_experiment->has_metadata("soma_encoding_version"));
Expand All @@ -303,15 +304,17 @@ TEST_CASE("SOMAExperiment: metadata") {
soma_experiment->close();

// md should not be available at (2, 2)
soma_experiment->open(OpenMode::read, TimestampRange(2, 2));
soma_experiment = SOMAExperiment::open(
uri, OpenMode::read, ctx, TimestampRange(2, 2));
REQUIRE(soma_experiment->metadata_num() == 2);
REQUIRE(soma_experiment->has_metadata("soma_object_type"));
REQUIRE(soma_experiment->has_metadata("soma_encoding_version"));
REQUIRE(!soma_experiment->has_metadata("md"));
soma_experiment->close();

// Metadata should also be retrievable in write mode
soma_experiment->open(OpenMode::write, TimestampRange(0, 2));
soma_experiment = SOMAExperiment::open(
uri, OpenMode::write, ctx, TimestampRange(0, 2));
REQUIRE(soma_experiment->metadata_num() == 3);
REQUIRE(soma_experiment->has_metadata("soma_object_type"));
REQUIRE(soma_experiment->has_metadata("soma_encoding_version"));
Expand All @@ -326,7 +329,8 @@ TEST_CASE("SOMAExperiment: metadata") {
soma_experiment->close();

// Confirm delete in read mode
soma_experiment->open(OpenMode::read, TimestampRange(0, 2));
soma_experiment = SOMAExperiment::open(
uri, OpenMode::read, ctx, TimestampRange(0, 2));
REQUIRE(!soma_experiment->has_metadata("md"));
REQUIRE(soma_experiment->metadata_num() == 2);
}
Expand All @@ -351,7 +355,8 @@ TEST_CASE("SOMAMeasurement: metadata") {
soma_measurement->close();

// Read metadata
soma_measurement->open(OpenMode::read, TimestampRange(0, 2));
soma_measurement = SOMAMeasurement::open(
uri, OpenMode::read, ctx, TimestampRange(0, 2));
REQUIRE(soma_measurement->metadata_num() == 3);
REQUIRE(soma_measurement->has_metadata("soma_object_type"));
REQUIRE(soma_measurement->has_metadata("soma_encoding_version"));
Expand All @@ -363,15 +368,17 @@ TEST_CASE("SOMAMeasurement: metadata") {
soma_measurement->close();

// md should not be available at (2, 2)
soma_measurement->open(OpenMode::read, TimestampRange(2, 2));
soma_measurement = SOMAMeasurement::open(
uri, OpenMode::read, ctx, TimestampRange(2, 2));
REQUIRE(soma_measurement->metadata_num() == 2);
REQUIRE(soma_measurement->has_metadata("soma_object_type"));
REQUIRE(soma_measurement->has_metadata("soma_encoding_version"));
REQUIRE(!soma_measurement->has_metadata("md"));
soma_measurement->close();

// Metadata should also be retrievable in write mode
soma_measurement->open(OpenMode::write, TimestampRange(0, 2));
soma_measurement = SOMAMeasurement::open(
uri, OpenMode::write, ctx, TimestampRange(0, 2));
REQUIRE(soma_measurement->metadata_num() == 3);
REQUIRE(soma_measurement->has_metadata("soma_object_type"));
REQUIRE(soma_measurement->has_metadata("soma_encoding_version"));
Expand All @@ -386,7 +393,8 @@ TEST_CASE("SOMAMeasurement: metadata") {
soma_measurement->close();

// Confirm delete in read mode
soma_measurement->open(OpenMode::read, TimestampRange(0, 2));
soma_measurement = SOMAMeasurement::open(
uri, OpenMode::read, ctx, TimestampRange(0, 2));
REQUIRE(!soma_measurement->has_metadata("md"));
REQUIRE(soma_measurement->metadata_num() == 2);
}
2 changes: 1 addition & 1 deletion libtiledbsoma/test/unit_soma_group.cc
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ std::tuple<std::string, uint64_t> create_array(
}

// Open array for writing
Array array(ctx, uri, TILEDB_WRITE, timestamp);
Array array(ctx, uri, TILEDB_WRITE, TemporalPolicy(TimeTravel, timestamp));
if (LOG_DEBUG_ENABLED()) {
array.schema().dump();
}
Expand Down

0 comments on commit d4138cc

Please sign in to comment.