TileDB icon indicating copy to clipboard operation
TileDB copied to clipboard

Add dump functions with `std::string*` argument

Open kounelisagis opened this issue 1 year ago • 4 comments

[sc-32959]

In TileDB-Py schema.dump() is called without any argument, leading writes to C stdout which Jupyter does not capture: https://github.com/TileDB-Inc/TileDB/blob/76dbda43d98bff7b71527fbffd0dfd657437b00f/tiledb/sm/array_schema/array_schema.cc#L693-L694 In stats, this method is already implemented: https://github.com/TileDB-Inc/TileDB/blob/76dbda43d98bff7b71527fbffd0dfd657437b00f/tiledb/sm/stats/global_stats.h#L171

Implementing a function that captures the output of the dump() to a string and then prints it to sys.stdout will fix the problem. Of course, ArraySchema::dump(std::string*) calls the dump() functions of other classes, so those need to be implemented as well.


TYPE: IMPROVEMENT DESC: Output of schema.dump() in TileDB-Py is not captured by Jupyter. Overloading the function to accept std::string* will give the ability to print the resulted string from Python.

kounelisagis avatar May 30 '24 09:05 kounelisagis

Even without dealing with the issues with Filter, there are a number of mostly-small changes to be made.

For class Filter, it looks like the following would work:

  1. A single definition of operator<< for filters.
  2. A pure virtual function Filter::output that (1) calls. This should be protected.
  3. An override of Filter::output in each class derived from filter. Also protected.
  4. A friend declaration for operator<< so that Filter::output is available.

@eric-hughes-tiledb, making it a friend causes the need to be in tiledb::sm, causing all other operator<< overloads to also be in the namespace.

kounelisagis avatar Jun 10 '24 18:06 kounelisagis

making it a friend causes the need to be in tiledb::sm

Friend declarations can be for namespace-qualified identifiers. friend ::something for the global namespace, for instance.

eric-hughes-tiledb avatar Jun 10 '24 18:06 eric-hughes-tiledb

Still reviewing...

eric-hughes-tiledb avatar Jun 18 '24 23:06 eric-hughes-tiledb

Thanks for all your comments!

I couldn't resolve all your suggestions. You can view the error I was getting from previous CIs. I'm not sure if there is a way they can be resolved. I am doing things (imports, using or not handles, etc.) that are widely done in the rest of the code.

kounelisagis avatar Jun 20 '24 11:06 kounelisagis