[c++/python] Address C++ warnings in Python build (#2581)

1. Add `TILEDB_REMOVE_DEPRECATIONS` as a CMake option that can be passed to the external TileDB build.
2. Update CI to build TileDB without the deprecated API.
3. Address a few misc. warnings in PyBind. 
   * Use `is` instead of `==` for PyBind type checks.
   * Use `TileDBSOMAPyError` instead of `TileDBSOMAError` when generating errors in `query_conditions.cc` (remove redefinition).
   * Remove unused variables.

Note: This PR does not prevent deprecated warnings in all CI. If TileDB is built downstream or downloaded, the deprecated API will still be included.

.github/workflows/libtiledb-ci.yml

@@ -19,7 +19,7 @@ jobs:
     - name: Checkout TileDB-SOMA
       uses: actions/checkout@v3
     - name: Build libTileDB-SOMA
-      run: TILEDBSOMA_COVERAGE="--coverage" ./scripts/bld
+      run: TILEDBSOMA_COVERAGE="--coverage" ./scripts/bld --no-tiledb-deprecated=true
     - name: Run libTileDB-SOMA unittests
       run: ctest --test-dir build/libtiledbsoma -C Release --verbose --rerun-failed --output-on-failur
     - name: Upload coverage to Codecov

.github/workflows/python-ci-packaging.yml

@@ -91,6 +91,7 @@ jobs:
             -D CMAKE_INSTALL_PREFIX:PATH=$(pwd)/external/ \
             -D OVERRIDE_INSTALL_PREFIX=OFF \
             -D DOWNLOAD_TILEDB_PREBUILT=OFF \
+            -D TILEDB_REMOVE_DEPRECATIONS=ON \
             -D FORCE_BUILD_TILEDB=OFF
           cmake --build build-libtiledbsoma -j $(nproc)
           cmake --build build-libtiledbsoma --target install-libtiledbsoma
@@ -186,6 +187,7 @@ jobs:
             -D CMAKE_INSTALL_PREFIX:PATH=$(pwd)/external/ \
             -D OVERRIDE_INSTALL_PREFIX=OFF \
             -D DOWNLOAD_TILEDB_PREBUILT=OFF \
+            -D TILEDB_REMOVE_DEPRECATIONS=ON \
             -D FORCE_BUILD_TILEDB=OFF
           cmake --build build-libtiledbsoma -j $(nproc)
           cmake --build build-libtiledbsoma --target install-libtiledbsoma
@@ -277,6 +279,7 @@ jobs:
             -D CMAKE_INSTALL_PREFIX:PATH=$(pwd)/external/ \
             -D OVERRIDE_INSTALL_PREFIX=OFF \
             -D DOWNLOAD_TILEDB_PREBUILT=OFF \
+            -D TILEDB_REMOVE_DEPRECATIONS=ON \
             -D FORCE_BUILD_TILEDB=OFF
           cmake --build build-libtiledbsoma -j 2
           cmake --build build-libtiledbsoma --target install-libtiledbsoma

.github/workflows/python-ci-single.yml

@@ -101,7 +101,7 @@ jobs:
 #        key: libtiledbsoma-build-dist-${{ inputs.os }}-${{ inputs.python_version }}-${{ hashFiles('libtiledbsoma', 'scripts/bld') }}
 
     - name: Install tiledbsoma
-      run: python -m pip -v install -e apis/python[dev]
+      run: python -m pip -v install -e apis/python[dev] -C "--build-option=--no-tiledb-deprecated"
       env:
         CC: ${{ inputs.cc }}
         CXX: ${{ inputs.cxx }}

.github/workflows/r-python-interop-testing.yml

@@ -80,7 +80,7 @@ jobs:
       #  run: cd apis/r && Rscript -e "options(bspm.version.check=TRUE); install.packages('tiledb', repos = c('https://eddelbuettel.r-universe.dev/bin/linux/jammy/4.3/', 'https://cloud.r-project.org'))"
 
       - name: Build and install libtiledbsoma
-        run: sudo scripts/bld --prefix=/usr/local && sudo ldconfig
+        run: sudo scripts/bld --prefix=/usr/local --no-tiledb-deprecated=true && sudo ldconfig
 
       - name: Install R-tiledbsoma
         run: |
@@ -100,7 +100,7 @@ jobs:
           cache-dependency-path: ./apis/python/setup.py
 
       - name: Install tiledbsoma
-        run: python -m pip -v install -e apis/python[dev]
+        run: python -m pip -v install -e apis/python[dev] -C "--build-option=--no-tiledb-deprecated"
 
       - name: Show Python package versions
         run: |

apis/python/setup.py

@@ -26,6 +26,7 @@
 
 try:
     from pybind11.setup_helpers import Pybind11Extension
+
 except ImportError:
     # Explanation:
     # https://pybind11.readthedocs.io/en/stable/compiling.html#classic-setup-requires
@@ -43,6 +44,7 @@
 
 tiledb_dir: Optional[pathlib.Path] = None
 tiledbsoma_dir: Optional[pathlib.Path] = None
+no_tiledb_dep: bool = False
 
 args = sys.argv[:]
 for arg in args:
@@ -53,6 +55,10 @@
     if (start, eq) == ("--libtiledbsoma", "="):
         tiledbsoma_dir = pathlib.Path(last)
         sys.argv.remove(arg)
+    if (start, eq) == ("--no-tiledb-deprecated", "="):
+        no_tiledb_dep = True
+        sys.argv.remove(arg)
+
 
 tiledb_dir = os.environ.get("TILEDB_PATH", tiledb_dir)
 tiledb_given = tiledb_dir is not None
@@ -167,10 +173,15 @@ def find_or_build_package_data(setuptools_cmd):
             bld_command = ["pwsh.exe", "./bld.ps1"]
             if tiledb_dir is not None:
                 bld_command.append(f"TileDBLocation={tiledb_dir}")
+            if no_tiledb_dep:
+                bld_command.append("RemoveTileDBDeprecated=ON")
+
         else:
             bld_command = ["./bld"]
             if tiledb_dir is not None:
                 bld_command.append(f"--tiledb={tiledb_dir}")
+            if no_tiledb_dep:
+                bld_command.append("--no-tiledb-deprecated=true")
 
         subprocess.run(bld_command, cwd=scripts_dir, check=True)
 

apis/python/src/tiledbsoma/common.cc

@@ -132,9 +132,9 @@ tiledb_datatype_t np_to_tdb_dtype(py::dtype type) {
         return _np_name_to_tdb_dtype[name];
 
     auto kind = py::str(py::getattr(type, "kind"));
-    if (kind == py::str("S"))
+    if (kind.is(py::str("S")))
         return TILEDB_STRING_ASCII;
-    if (kind == py::str("U"))
+    if (kind.is(py::str("U")))
         return TILEDB_STRING_UTF8;
 
     TPY_ERROR_LOC("could not handle numpy dtype");

apis/python/src/tiledbsoma/query_condition.cc

@@ -32,8 +32,6 @@
 
 #include "common.h"
 
-#define TPY_ERROR_LOC(m) throw tiledbsoma::TileDBSOMAError(m);
-
 #if TILEDB_VERSION_MAJOR == 2 && TILEDB_VERSION_MINOR >= 2
 #if !defined(NDEBUG)
 #endif

apis/python/src/tiledbsoma/reindexer.cc

@@ -54,7 +54,6 @@ py::array_t<int64_t> get_indexer_general(
     size_t size = input_buffer.shape[0];
     auto results = py::array_t<int64_t>(size);
     auto results_buffer = results.request();
-    size_t results_size = results_buffer.shape[0];
     int64_t* results_ptr = static_cast<int64_t*>(results_buffer.ptr);
     indexer.lookup(input_ptr, results_ptr, size);
     return results;
@@ -113,7 +112,6 @@ py::array_t<int64_t> get_indexer_py_arrow(
     // Allocate the output
     auto results = py::array_t<int64_t>(total_size);
     auto results_buffer = results.request();
-    size_t results_size = results_buffer.shape[0];
     int64_t* results_ptr = static_cast<int64_t*>(results_buffer.ptr);
 
     // Write output (one chunk at a time)
@@ -139,9 +137,6 @@ void load_reindexer(py::module& m) {
         .def(
             "map_locations",
             [](IntIndexer& indexer, py::array_t<int64_t> keys) {
-                auto buffer = keys.request();
-                int64_t* data = static_cast<int64_t*>(buffer.ptr);
-                size_t length = buffer.shape[0];
                 indexer.map_locations(keys.data(), keys.size());
             })
         // Perform lookup for a large input array of keys and writes the

apis/python/src/tiledbsoma/soma_dataframe.cc

@@ -68,7 +68,7 @@ void load_soma_dataframe(py::module& m) {
                         auto val = metadata.attr("get")(
                             py::str(child->name).attr("encode")("utf-8"));
 
-                        if (val != py::none() &&
+                        if (!val.is(py::none()) &&
                             val.cast<std::string>() == "nullable") {
                             child->flags |= ARROW_FLAG_NULLABLE;
                         } else {
@@ -133,4 +133,4 @@ void load_soma_dataframe(py::module& m) {
             "index_column_names", &SOMADataFrame::index_column_names)
         .def_property_readonly("count", &SOMADataFrame::count);
 }
-}  // namespace libtiledbsomacpp
+}  // namespace libtiledbsomacpp

libtiledbsoma/CMakeLists.txt

@@ -60,6 +60,7 @@ option(TILEDB_AZURE "Enables Azure Storage support using azure-storage-cpp" ON)
 option(TILEDB_GCS "Enables GCS Storage support using google-cloud-cpp" OFF)
 option(TILEDB_HDFS "Enables HDFS support using the official Hadoop JNI bindings" OFF)
 option(TILEDB_WERROR "Enables the -Werror flag during compilation." OFF)
+option(TILEDB_REMOVE_DEPRECATIONS "If true, do not build deprecated APIs." OFF)
 option(TILEDB_SERIALIZATION "If true, enables building with support for query serialization" ON)
 option(TILEDB_VERBOSE "If true, sets default logging to verbose for TileDB" OFF)
 option(OVERRIDE_INSTALL_PREFIX "Ignores the setting of CMAKE_INSTALL_PREFIX and sets a default prefix" ON)

libtiledbsoma/cmake/Modules/FindTileDB_EP.cmake

@@ -125,6 +125,7 @@ else()
             -DTILEDB_HDFS=${TILEDB_HDFS}
             -DTILEDB_SERIALIZATION=${TILEDB_SERIALIZATION}
             -DTILEDB_WERROR=${TILEDB_WERROR}
+            -DTILEDB_REMOVE_DEPRECATIONS=${TILEDB_REMOVE_DEPRECATIONS}
             -DTILEDB_VERBOSE=${TILEDB_VERBOSE}
             -DTILEDB_TESTS=OFF
             -DCMAKE_BUILD_TYPE=$<CONFIG>

scripts/bld

@@ -12,13 +12,15 @@ build="Release"
 prefix=""
 tiledb=""
 cmake_verbose="false"
+no_tiledb_deprecated="false"
 
 while test $# != 0; do
   case "$1" in
   --build=*) build=$(arg "$1");;
   --prefix=*) prefix=$(arg "$1");;
   --tiledb=*) tiledb=$(arg "$1");;
   --cmake-verbose=*) cmake_verbose=$(arg "$1");;
+  --no-tiledb-deprecated=*) no_tiledb_deprecated=$(arg "$1");;
   esac
   shift
 done
@@ -53,16 +55,20 @@ if [ "$cmake_verbose" = "true" ]; then
 
   # TILEDB_WERROR=OFF is necessary to build core with XCode 14; doesn't hurt for XCode 13.
   extra_opts+=" -DCMAKE_VERBOSE_MAKEFILE:BOOL=ON -DTILEDB_WERROR=OFF -DTILEDBSOMA_ENABLE_WERROR=OFF"
-  
+
   # Debug cmake find commands
   extra_opts+=" --debug-find"
 
   # Also (pro-tip), set nproc=1 to get a more deterministic ordering of output lines.
   nproc=1
 fi
 
+if [ "$no_tiledb_deprecated" = "true" ]; then
+    extra_opts+=" -DTILEDB_REMOVE_DEPRECATIONS=ON"
+fi
+
 # set installation path
-if [ -n "${prefix}"  ]; then 
+if [ -n "${prefix}"  ]; then
   extra_opts+=" -DCMAKE_INSTALL_PREFIX=${prefix} -DOVERRIDE_INSTALL_PREFIX=OFF"
 fi
 

scripts/bld.ps1

@@ -31,6 +31,7 @@ Param(
     [string]$Configuration = 'Release',
     [string]$Prefix = '',
     [string]$TileDBLocation = '',
+    [string]$TileDBRemoveDeprecations = '',
     [Alias('J')]
     [int]
     $BuildProcesses = $env:NUMBER_OF_PROCESSORS
@@ -52,6 +53,10 @@ if ($TileDBLocation -ne '') {
     $ExtraOpts += " -DTileDB_DIR=$TileDBLocation -DFORCE_BUILD_TILEDB=OFF"
 }
 
+if ($RemoveTileDBDeprecated -ne '') {
+    $ExtraOpts += " -DTILEDB_REMOVE_DEPRECATIONS=$TileDBRemoveDeprecations"
+}
+
 Write-Host "Building $Configuration build"
 
 $BuildDir = (Get-Item -Path $PSScriptRoot).Parent.FullName + '\build'