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

[c++] Fix Windows build regressions and stop using deprecated TileDB APIs #2445

Merged
merged 3 commits into from
Apr 17, 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
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 @@ -121,7 +121,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 @@ -120,7 +120,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 @@ -97,8 +97,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 @@ -97,8 +97,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 @@ -120,7 +120,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 @@ -452,8 +452,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 @@ -55,7 +55,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 @@ -314,7 +314,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 @@ -326,15 +327,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 @@ -349,7 +352,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 @@ -367,7 +371,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 @@ -379,15 +384,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 @@ -402,7 +409,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
Loading