Fix stats_dump broken return value type (#2104)

* Raise error if stats_enable is not called before stats_dump
* Fix stats dump return type

tiledb/core.cc

@@ -1594,6 +1594,8 @@ void init_stats() {
 
 void disable_stats() { g_stats.reset(nullptr); }
 
+bool stats_enabled() { return (bool)g_stats; }
+
 void increment_stat(std::string key, double value) {
   auto &stats_counters = g_stats.get()->counters;
 
@@ -1759,6 +1761,7 @@ void init_core(py::module &m) {
     Stats::disable();
     disable_stats();
   });
+  m.def("stats_enabled", &stats_enabled);
   m.def("reset_stats", []() {
     Stats::reset();
     init_stats();

tiledb/stats.py

@@ -1,6 +1,8 @@
 from json import dumps as json_dumps
 from json import loads as json_loads
 
+from tiledb import TileDBError
+
 
 def stats_enable():
     """Enable TileDB internal statistics."""
@@ -34,9 +36,12 @@ def stats_dump(
     :param json: Return stats JSON object (default: False)
     :param verbose: Print extended internal statistics (default: True)
     """
-    from .main import stats_dump_str, stats_raw_dump_str
+    from .main import stats_dump_str, stats_enabled, stats_raw_dump_str
 
-    stats_str = None
+    if not stats_enabled():
+        raise TileDBError(
+            "Statistics are not enabled. Call tiledb.stats_enable() first."
+        )
 
     if json or not verbose:
         stats_str = stats_raw_dump_str()
@@ -50,9 +55,9 @@ def stats_dump(
         if include_python:
             from .main import python_internal_stats
 
-            stats_json_core["python"] = json_dumps(python_internal_stats(True))
+            stats_json_core["python"] = python_internal_stats(True)
         if json:
-            return stats_json_core
+            return json_dumps(stats_json_core)
 
     stats_str = ""
 

tiledb/tests/test_fixes.py

@@ -1,5 +1,6 @@
 import concurrent
 import concurrent.futures
+import json
 import os
 import subprocess
 import sys
@@ -172,6 +173,61 @@ def test_sc16301_arrow_extra_estimate_dense(self):
                 )
             tiledb.stats_disable()
 
+    def test_sc58286_fix_stats_dump_return_value_broken(self):
+        uri = self.path("test_sc58286_fix_stats_dump_return_value_broken")
+        dim1 = tiledb.Dim(name="d1", dtype="int64", domain=(1, 3))
+        att = tiledb.Attr(name="a1", dtype="<U0", var=True)
+
+        schema = tiledb.ArraySchema(
+            domain=tiledb.Domain(dim1),
+            attrs=(att,),
+            sparse=False,
+            allows_duplicates=False,
+        )
+        tiledb.Array.create(uri, schema)
+
+        with tiledb.open(uri, "w") as A:
+            A[:] = np.array(["aaa", "bb", "c"])
+
+        with tiledb.open(uri) as A:
+            tiledb.stats_enable()
+            A[:]
+
+            # check that the stats cannot be parsed as json
+            stats = tiledb.stats_dump(print_out=False, json=False)
+            assert isinstance(stats, str)
+            with pytest.raises(json.decoder.JSONDecodeError):
+                json.loads(stats)
+
+            stats = tiledb.stats_dump(print_out=False, json=False, include_python=True)
+            assert isinstance(stats, str)
+            with pytest.raises(json.decoder.JSONDecodeError):
+                json.loads(stats)
+
+            # check that the stats can be parsed as json
+            stats = tiledb.stats_dump(print_out=False, json=True)
+            assert isinstance(stats, str)
+            json.loads(stats)
+
+            stats = tiledb.stats_dump(print_out=False, json=True, include_python=True)
+            assert isinstance(stats, str)
+            res = json.loads(stats)
+
+            tiledb.stats_disable()
+
+            # check that some fields are present in the json output and are of the correct type
+            assert "counters" in res and isinstance(res["counters"], dict)
+            assert "timers" in res and isinstance(res["timers"], dict)
+            assert "python" in res and isinstance(res["python"], dict)
+
+    def test_fix_stats_error_messages(self):
+        # Test that stats_dump prints a user-friendly error message when stats are not enabled
+        with pytest.raises(tiledb.TileDBError) as exc:
+            tiledb.stats_dump()
+        assert "Statistics are not enabled. Call tiledb.stats_enable() first." in str(
+            exc.value
+        )
+
     @pytest.mark.skipif(
         not has_pandas() and has_pyarrow(),
         reason="pandas>=1.0,<3.0 or pyarrow>=1.0 not installed",

tiledb/tests/test_stats.py

@@ -64,7 +64,7 @@ def test_stats_include_python_json(self):
             tiledb.stats_enable()
             assert_array_equal(T, np.arange(10))
             json_stats = tiledb.stats_dump(print_out=False, json=True)
-            assert isinstance(json_stats, dict)
+            assert isinstance(json_stats, str)
             assert "python" in json_stats
             assert "timers" in json_stats
             assert "counters" in json_stats