[c++] Remove spdlog requirement (#1852)

* Keep logger.h internal; do not install * Restore conditional include of carrow.h --------- Co-authored-by: Dirk Eddelbuettel <[email protected]>
single-cell-data · Nov 3, 2023 · af7f1a0 · af7f1a0
1 parent 68a0602
commit af7f1a0
Show file tree

Hide file tree

Showing 15 changed files with 406 additions and 314 deletions.
diff --git a/apis/python/src/tiledbsoma/pytiledbsoma.cc b/apis/python/src/tiledbsoma/pytiledbsoma.cc
@@ -474,10 +474,9 @@ PYBIND11_MODULE(pytiledbsoma, m) {
                         reader.set_dim_points(
                             dim, coords.cast<std::vector<std::string>>());
                     } else {
-                        throw TileDBSOMAError(fmt::format(
-                            "[pytiledbsoma] set_dim_points: type={} not "
-                            "supported",
-                            arrow_schema.format));
+                        throw TileDBSOMAError(
+                            "[pytiledbsoma] set_dim_points: type=" + std::string(arrow_schema.format) + " not "
+                            "supported");
                     }
 
                     // Release arrow schema

diff --git a/libtiledbsoma/src/CMakeLists.txt b/libtiledbsoma/src/CMakeLists.txt
@@ -52,7 +52,9 @@ add_library(TILEDB_SOMA_OBJECTS OBJECT
   ${CMAKE_CURRENT_SOURCE_DIR}/soma/soma_dataframe.cc
   ${CMAKE_CURRENT_SOURCE_DIR}/soma/soma_dense_ndarray.cc
   ${CMAKE_CURRENT_SOURCE_DIR}/soma/soma_sparse_ndarray.cc
+  ${CMAKE_CURRENT_SOURCE_DIR}/soma/array_buffers.cc
   ${CMAKE_CURRENT_SOURCE_DIR}/soma/column_buffer.cc
+  ${CMAKE_CURRENT_SOURCE_DIR}/utils/arrow_adapter.cc
   ${CMAKE_CURRENT_SOURCE_DIR}/utils/logger.cc
   ${CMAKE_CURRENT_SOURCE_DIR}/utils/stats.cc
   ${CMAKE_CURRENT_SOURCE_DIR}/utils/util.cc
@@ -201,7 +203,6 @@ install(FILES
   ${CMAKE_CURRENT_SOURCE_DIR}/utils/arrow_adapter.h  
   ${CMAKE_CURRENT_SOURCE_DIR}/utils/carrow.h  
   ${CMAKE_CURRENT_SOURCE_DIR}/utils/common.h  
-  ${CMAKE_CURRENT_SOURCE_DIR}/utils/logger.h  
   ${CMAKE_CURRENT_SOURCE_DIR}/utils/stats.h  
   ${CMAKE_CURRENT_SOURCE_DIR}/utils/util.h  
   ${CMAKE_CURRENT_SOURCE_DIR}/utils/version.h

diff --git a/libtiledbsoma/src/cli/cli.cc b/libtiledbsoma/src/cli/cli.cc
@@ -33,6 +33,8 @@
 #include "soma/enums.h"
 #include "soma/soma_array.h"
 #include "utils/arrow_adapter.h"
+#include "utils/carrow.h"
+#include "utils/logger.h"
 
 using namespace tiledbsoma;
 

diff --git a/libtiledbsoma/src/soma/array_buffers.cc b/libtiledbsoma/src/soma/array_buffers.cc
@@ -0,0 +1,58 @@
+/**
+ * @file   array_buffers.cc
+ *
+ * @section LICENSE
+ *
+ * The MIT License
+ *
+ * @copyright Copyright (c) 2022 TileDB, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ * @section DESCRIPTION
+ *
+ * This file defines the ArrayBuffers class.
+ */
+
+#include "array_buffers.h"
+#include "../utils/logger.h"
+
+namespace tiledbsoma {
+
+using namespace tiledb;
+
+std::shared_ptr<ColumnBuffer> ArrayBuffers::at(const std::string& name) {
+    if (!contains(name)) {
+        throw TileDBSOMAError(
+            fmt::format("[ArrayBuffers] column '{}' does not exist", name));
+    }
+    return buffers_[name];
+}
+
+void ArrayBuffers::emplace(
+    const std::string& name, std::shared_ptr<ColumnBuffer> buffer) {
+    if (contains(name)) {
+        throw TileDBSOMAError(
+            fmt::format("[ArrayBuffers] column '{}' already exists", name));
+    }
+    names_.push_back(name);
+    buffers_.emplace(name, buffer);
+}
+
+}  // namespace tiledbsoma
diff --git a/libtiledbsoma/src/soma/array_buffers.h b/libtiledbsoma/src/soma/array_buffers.h
@@ -38,7 +38,6 @@
 #include <tiledb/tiledb>
 
 #include "../utils/common.h"
-#include "../utils/logger.h"
 #include "column_buffer.h"
 
 namespace tiledbsoma {
@@ -58,13 +57,7 @@ class ArrayBuffers {
      * @param name Column name
      * @return std::shared_ptr<ColumnBuffer> Column buffer
      */
-    std::shared_ptr<ColumnBuffer> at(const std::string& name) {
-        if (!contains(name)) {
-            throw TileDBSOMAError(
-                fmt::format("[ArrayBuffers] column '{}' does not exist", name));
-        }
-        return buffers_[name];
-    }
+    std::shared_ptr<ColumnBuffer> at(const std::string& name);
 
     /**
      * @brief Return true if a buffer with the given name exists.
@@ -83,15 +76,7 @@ class ArrayBuffers {
      * @param name Column name
      * @param buffer Column buffer
      */
-    void emplace(
-        const std::string& name, std::shared_ptr<ColumnBuffer> buffer) {
-        if (contains(name)) {
-            throw TileDBSOMAError(
-                fmt::format("[ArrayBuffers] column '{}' already exists", name));
-        }
-        names_.push_back(name);
-        buffers_.emplace(name, buffer);
-    }
+    void emplace(const std::string& name, std::shared_ptr<ColumnBuffer> buffer);
 
     /**
      * @brief Returns the ordered vector of names.

diff --git a/libtiledbsoma/src/soma/column_buffer.cc b/libtiledbsoma/src/soma/column_buffer.cc
@@ -31,6 +31,7 @@
  */
 
 #include "column_buffer.h"
+#include "../utils/logger.h"
 
 namespace tiledbsoma {
 
@@ -152,6 +153,10 @@ ColumnBuffer::ColumnBuffer(
     }
 }
 
+ColumnBuffer::~ColumnBuffer() {
+    LOG_TRACE(fmt::format("[ColumnBuffer] release '{}'", name_));
+}
+
 void ColumnBuffer::attach(Query& query) {
     // We cannot use:
     // `set_data_buffer(const std::string& name, std::vector<T>& buf)`

diff --git a/libtiledbsoma/src/soma/column_buffer.h b/libtiledbsoma/src/soma/column_buffer.h
@@ -39,7 +39,6 @@
 #include <tiledb/tiledb_experimental>
 
 #include "../utils/common.h"
-#include "../utils/logger.h"
 #include "span/span.hpp"
 
 namespace tiledbsoma {
@@ -128,9 +127,7 @@ class ColumnBuffer {
     ColumnBuffer(const ColumnBuffer&) = delete;
     ColumnBuffer(ColumnBuffer&&) = default;
 
-    ~ColumnBuffer() {
-        LOG_TRACE(fmt::format("[ColumnBuffer] release '{}'", name_));
-    }
+    ~ColumnBuffer();
 
     /**
      * @brief Attach this ColumnBuffer to a TileDB query.

diff --git a/libtiledbsoma/src/soma/managed_query.cc b/libtiledbsoma/src/soma/managed_query.cc
@@ -33,7 +33,7 @@
 #include "managed_query.h"
 #include <tiledb/array_experimental.h>
 #include <tiledb/attribute_experimental.h>
-#include "logger_public.h"
+#include "../utils/logger.h"
 #include "utils/common.h"
 namespace tiledbsoma {
 
@@ -220,4 +220,13 @@ std::shared_ptr<ArrayBuffers> ManagedQuery::submit_read() {
     return buffers_;
 }
 
+void ManagedQuery::check_column_name(const std::string& name) {
+    if (!buffers_->contains(name)) {
+        throw TileDBSOMAError(fmt::format(
+            "[ManagedQuery] Column '{}' is not available in the query "
+            "results.",
+            name));
+    }
+}
+
 };  // namespace tiledbsoma
diff --git a/libtiledbsoma/src/soma/managed_query.h b/libtiledbsoma/src/soma/managed_query.h
@@ -429,14 +429,7 @@ class ManagedQuery {
      *
      * @param name Column name
      */
-    void check_column_name(const std::string& name) {
-        if (!buffers_->contains(name)) {
-            throw TileDBSOMAError(fmt::format(
-                "[ManagedQuery] Column '{}' is not available in the query "
-                "results.",
-                name));
-        }
-    }
+    void check_column_name(const std::string& name);
 
     // TileDB array being queried.
     std::shared_ptr<Array> array_;

diff --git a/libtiledbsoma/src/soma/soma_array.cc b/libtiledbsoma/src/soma/soma_array.cc
@@ -32,8 +32,8 @@
 
 #include "soma_array.h"
 #include <tiledb/array_experimental.h>
+#include "../utils/logger.h"
 #include "../utils/util.h"
-#include "logger_public.h"
 namespace tiledbsoma {
 using namespace tiledb;
 

diff --git a/libtiledbsoma/src/soma/soma_array.h b/libtiledbsoma/src/soma/soma_array.h
@@ -40,6 +40,7 @@
 #include <tiledb/tiledb>
 #include <tiledb/tiledb_experimental>
 #include "enums.h"
+#include "logger_public.h"
 #include "managed_query.h"
 
 namespace tiledbsoma {
@@ -251,12 +252,13 @@ class SOMAArray {
         int partition_count) {
         // Validate partition inputs
         if (partition_index >= partition_count) {
-            throw TileDBSOMAError(fmt::format(
-                "[SOMAArray] partition_index ({}) must be < "
-                "partition_count "
-                "({})",
-                partition_index,
-                partition_count));
+            // TODO this use to be formatted with fmt::format which is part of
+            // internal header spd/log/fmt/fmt.h and should not be used.
+            // In C++20, this can be replaced with std::format.
+            std::ostringstream err;
+            err << "[SOMAArray] partition_index (" << partition_index
+                << ") must be < partition_count (" << partition_count;
+            throw TileDBSOMAError(err.str());
         }
 
         if (partition_count > 1) {
@@ -268,19 +270,17 @@ class SOMAArray {
                 partition_size = points.size() - start;
             }
 
-            LOG_DEBUG(fmt::format(
-                "[SOMAArray] set_dim_points partitioning: sizeof(T)={} "
-                "dim={} "
-                "index={} "
-                "count={} "
-                "range=[{}, {}] of {} points",
-                sizeof(T),
-                dim,
-                partition_index,
-                partition_count,
-                start,
-                start + partition_size - 1,
-                points.size()));
+            // TODO this use to be formatted with fmt::format which is part of
+            // internal header spd/log/fmt/fmt.h and should not be used.
+            // In C++20, this can be replaced with std::format.
+            std::ostringstream log_dbg;
+            log_dbg << "[SOMAArray] set_dim_points partitioning:"
+                    << " sizeof(T)=" << sizeof(T) << " dim=" << dim
+                    << " index=" << partition_index
+                    << " count=" << partition_count << " range =[" << start
+                    << ", " << start + partition_size - 1 << "] of "
+                    << points.size() << "points";
+            LOG_DEBUG(log_dbg.str());
 
             mq_->select_points(
                 dim, tcb::span<T>{&points[start], partition_size});
@@ -301,7 +301,8 @@ class SOMAArray {
     template <typename T>
     void set_dim_points(const std::string& dim, const std::vector<T>& points) {
         LOG_DEBUG(
-            fmt::format("[SOMAArray] set_dim_points: sizeof(T)={}", sizeof(T)));
+            "[SOMAArray] set_dim_points: sizeof(T)=" +
+            std::to_string(sizeof(T)));
         mq_->select_points(dim, points);
     }