Move fragment list consolidation API to pybind #1999
Conversation
tiledb/cc/array.cc
Outdated
| .def("consolidate", | ||
| py::overload_cast<const Context &, const std::string &, | ||
| Config *const>(&Array::consolidate), | ||
| py::call_guard<py::gil_scoped_release>()) | ||
| [](Array &self) { | ||
| if (self.query_type() == TILEDB_READ) { | ||
| throw TileDBError("cannot consolidate array opened in readonly " | ||
| "mode (mode='r')"); | ||
| } | ||
|
|
||
| Config config; | ||
| Context ctx; | ||
| Array::consolidate(ctx, self.uri(), &config); | ||
| }) |
There was a problem hiding this comment.
I don't think we should have this one. The default context should be coming from the already existing tiledb.default_ctx() rather than constructing a new tiledb::Context.
There was a problem hiding this comment.
But if we delete this, we cannot have calls like array.consolidate() without any arguments.
This can be done with current implementation.
tiledb/tests/test_libtiledb.py
Outdated
| config = tiledb.Config() | ||
|
|
||
| ctx = tiledb.Ctx(config=config) | ||
| lt_array = lt.Array(ctx, path, lt.QueryType.READ) |
There was a problem hiding this comment.
Ah ok I did not catch this before. The underlying tiledb.cc (lt) bindings are not meant to be public. The user should still be using tiledb.Array. For example, tiledb.ArraySchema inherits from lt.Schema. And this way we can set the default ctx to tiledb.default_ctx().
There was a problem hiding this comment.
What I think is that the static tiledb.consolidate method can be in Pybind11, but we still need the Cython tiledb.libtiledb.Array.consolidate method. Otherwise, to create a pure Python Array class as we did for ArraySchema, you would need to move everything in the Array class out of Cython and into Pybind11 which is a TON of work.
There was a problem hiding this comment.
That's exactly the problem. That's the reason that it wasn't clear what to do with that story.
kounelisagis
deleted the
agis/sc-44909/move-fragment-list-consolidation-api-to-pybind
branch
No description provided.