Skip to content

Commit f40530f

Browse files
Replace dump functions with operator<< overloads (#5026)
[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 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. **But** we don't even want to write functions called dump internally; that's a C idiom; we will reserve that term for the C API only. Since we're dealing with text output, everything here can be done better by overloading `operator<<` for `std::ostream`. --- TYPE: IMPROVEMENT DESC: Output of `schema.dump()` in TileDB-Py is not captured by Jupyter. Replacing `dump` functions with `operator<<` overloads will give the ability to print the resulted string from Python. --------- Co-authored-by: Eric Hughes <[email protected]>
1 parent cdfa6ff commit f40530f

File tree

88 files changed

+858
-563
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

88 files changed

+858
-563
lines changed

test/regression/targets/sc-38300.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ void create_array(const std::string& array_uri) {
5353
void dump_schema(const std::string& array_uri) {
5454
Context ctx;
5555
Array array(ctx, array_uri, TILEDB_READ);
56-
array.schema().dump();
56+
std::cout << array.schema();
5757
ArrayExperimental::load_all_enumerations(ctx, array);
58-
array.schema().dump();
58+
std::cout << array.schema();
5959
}

test/src/unit-cppapi-enumerations.cc

Lines changed: 2 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -674,27 +674,10 @@ CPPEnumerationFx::~CPPEnumerationFx() {
674674

675675
template <typename T>
676676
void CPPEnumerationFx::check_dump(const T& val) {
677-
FILE* handle = fopen(dump_name.c_str(), "w");
678-
REQUIRE(handle != nullptr);
679-
val.dump(handle);
680-
fclose(handle);
681-
682677
std::stringstream ss;
678+
ss << val;
683679

684-
// Scoped in an anonymous block to ensure that rstream closes before
685-
// we attempt to delete the file for cleanup.
686-
{
687-
std::ifstream rstream(dump_name);
688-
if (rstream.is_open()) {
689-
ss << rstream.rdbuf();
690-
}
691-
}
692-
693-
auto data = ss.str();
694-
auto iter = data.find("Enumeration");
695-
REQUIRE(iter != std::string::npos);
696-
697-
vfs_.remove_file(dump_name);
680+
REQUIRE(ss.str().find("Enumeration") != std::string::npos);
698681
}
699682

700683
void CPPEnumerationFx::create_array(bool with_empty_enumeration) {

test/src/unit-cppapi-fill_values.cc

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -40,23 +40,13 @@
4040
using namespace tiledb;
4141

4242
void check_dump(const Attribute& attr, const std::string& gold_out) {
43-
FILE* gold_fout = fopen("gold_fout.txt", "w");
44-
fwrite(gold_out.c_str(), sizeof(char), gold_out.size(), gold_fout);
45-
fclose(gold_fout);
46-
FILE* fout = fopen("fout.txt", "w");
47-
attr.dump(fout);
48-
fclose(fout);
49-
#ifdef _WIN32
50-
CHECK(!system("FC gold_fout.txt fout.txt > nul"));
51-
#else
52-
CHECK(!system("diff gold_fout.txt fout.txt"));
53-
#endif
43+
std::stringstream ss;
44+
ss << attr;
45+
CHECK(gold_out == ss.str());
5446

5547
// Clean up
5648
Context ctx;
5749
VFS vfs(ctx);
58-
CHECK_NOTHROW(vfs.remove_file("gold_fout.txt"));
59-
CHECK_NOTHROW(vfs.remove_file("fout.txt"));
6050
}
6151

6252
void create_array_1d(

tiledb/api/c_api/attribute/attribute_api.cc

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
*
66
* The MIT License
77
*
8-
* @copyright Copyright (c) 2023 TileDB, Inc.
8+
* @copyright Copyright (c) 2023-2024 TileDB, Inc.
99
*
1010
* Permission is hereby granted, free of charge, to any person obtaining a copy
1111
* of this software and associated documentation files (the "Software"), to deal
@@ -135,7 +135,28 @@ int32_t tiledb_attribute_get_cell_size(
135135
int32_t tiledb_attribute_dump(
136136
const tiledb_attribute_handle_t* attr, FILE* out) {
137137
ensure_attribute_is_valid(attr);
138-
attr->dump(out);
138+
ensure_cstream_handle_is_valid(out);
139+
140+
std::stringstream ss;
141+
ss << *attr;
142+
size_t r = fwrite(ss.str().c_str(), sizeof(char), ss.str().size(), out);
143+
if (r != ss.str().size()) {
144+
throw CAPIException(
145+
"Error writing attribute " + attr->name() + " to output stream");
146+
}
147+
148+
return TILEDB_OK;
149+
}
150+
151+
int32_t tiledb_attribute_dump_str(
152+
const tiledb_attribute_handle_t* attr, tiledb_string_handle_t** out) {
153+
ensure_attribute_is_valid(attr);
154+
ensure_output_pointer_is_valid(out);
155+
156+
std::stringstream ss;
157+
ss << *attr;
158+
*out = tiledb_string_handle_t::make_handle(ss.str());
159+
139160
return TILEDB_OK;
140161
}
141162

@@ -186,7 +207,7 @@ capi_return_t tiledb_attribute_set_enumeration_name(
186207
}
187208

188209
capi_return_t tiledb_attribute_get_enumeration_name(
189-
tiledb_attribute_t* attr, tiledb_string_t** name) {
210+
tiledb_attribute_t* attr, tiledb_string_handle_t** name) {
190211
ensure_attribute_is_valid(attr);
191212
ensure_output_pointer_is_valid(name);
192213

@@ -203,6 +224,12 @@ capi_return_t tiledb_attribute_get_enumeration_name(
203224

204225
} // namespace tiledb::api
205226

227+
std::ostream& operator<<(
228+
std::ostream& os, const tiledb_attribute_handle_t& attr) {
229+
os << *attr.attr_;
230+
return os;
231+
}
232+
206233
using tiledb::api::api_entry_context;
207234

208235
CAPI_INTERFACE(
@@ -308,6 +335,15 @@ CAPI_INTERFACE(
308335
return api_entry_context<tiledb::api::tiledb_attribute_dump>(ctx, attr, out);
309336
}
310337

338+
CAPI_INTERFACE(
339+
attribute_dump_str,
340+
tiledb_ctx_t* ctx,
341+
const tiledb_attribute_handle_t* attr,
342+
tiledb_string_handle_t** out) {
343+
return api_entry_context<tiledb::api::tiledb_attribute_dump_str>(
344+
ctx, attr, out);
345+
}
346+
311347
CAPI_INTERFACE(
312348
attribute_set_fill_value,
313349
tiledb_ctx_t* ctx,
@@ -365,7 +401,7 @@ CAPI_INTERFACE(
365401
attribute_get_enumeration_name,
366402
tiledb_ctx_t* ctx,
367403
tiledb_attribute_t* attr,
368-
tiledb_string_t** name) {
404+
tiledb_string_handle_t** name) {
369405
return api_entry_context<tiledb::api::tiledb_attribute_get_enumeration_name>(
370406
ctx, attr, name);
371407
}

tiledb/api/c_api/attribute/attribute_api_external.h

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,31 @@ TILEDB_EXPORT int32_t tiledb_attribute_dump(
310310
const tiledb_attribute_t* attr,
311311
FILE* out) TILEDB_NOEXCEPT;
312312

313+
/**
314+
* Dumps the contents of an Attribute in ASCII form to the selected string
315+
* output.
316+
*
317+
* The output string handle must be freed by the user after use.
318+
*
319+
* **Example:**
320+
*
321+
* @code{.c}
322+
* tiledb_string_t* tdb_string;
323+
* tiledb_attribute_dump_str(ctx, attr, &tdb_string);
324+
* // Use the string
325+
* tiledb_string_free(&tdb_string);
326+
* @endcode
327+
*
328+
* @param ctx The TileDB context.
329+
* @param attr The attribute.
330+
* @param out The output string.
331+
* @return `TILEDB_OK` for success and `TILEDB_ERR` for error.
332+
*/
333+
TILEDB_EXPORT int32_t tiledb_attribute_dump_str(
334+
tiledb_ctx_t* ctx,
335+
const tiledb_attribute_t* attr,
336+
tiledb_string_t** out) TILEDB_NOEXCEPT;
337+
313338
/**
314339
* Sets the default fill value for the input attribute. This value will
315340
* be used for the input attribute whenever querying (1) an empty cell in

tiledb/api/c_api/attribute/attribute_api_internal.h

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
*
66
* The MIT License
77
*
8-
* @copyright Copyright (c) 2023 TileDB, Inc.
8+
* @copyright Copyright (c) 2023-2024 TileDB, Inc.
99
*
1010
* Permission is hereby granted, free of charge, to any person obtaining a copy
1111
* of this software and associated documentation files (the "Software"), to deal
@@ -33,7 +33,6 @@
3333
#ifndef TILEDB_CAPI_ATTRIBUTE_INTERNAL_H
3434
#define TILEDB_CAPI_ATTRIBUTE_INTERNAL_H
3535

36-
#include "attribute_api_external.h"
3736
#include "tiledb/api/c_api_support/handle/handle.h"
3837
#include "tiledb/common/common.h"
3938
#include "tiledb/sm/array_schema/attribute.h"
@@ -90,7 +89,7 @@ struct tiledb_attribute_handle_t
9089
/**
9190
* Copy the underlying attribute object.
9291
*/
93-
[[nodiscard]] attribute_type copy_attribute() {
92+
[[nodiscard]] attribute_type copy_attribute() const {
9493
return attr_;
9594
}
9695

@@ -190,13 +189,11 @@ struct tiledb_attribute_handle_t
190189
[[nodiscard]] std::optional<std::string> get_enumeration_name() const {
191190
return attr_->get_enumeration_name();
192191
};
193-
194192
/**
195193
* Facade for `Attribute` function
196194
*/
197-
void dump(FILE* out) const {
198-
attr_->dump(out);
199-
}
195+
friend std::ostream& operator<<(
196+
std::ostream& os, const tiledb_attribute_handle_t& attr);
200197
};
201198

202199
namespace tiledb::api {

tiledb/api/c_api/attribute/test/unit_capi_attribute.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
#include "../../../c_api_test_support/testsupport_capi_context.h"
3434
#include "../../../c_api_test_support/testsupport_capi_datatype.h"
3535
#include "../../filter_list/filter_list_api_internal.h"
36-
#include "../attribute_api_internal.h"
36+
#include "../attribute_api_external.h"
3737
using namespace tiledb::api::test_support;
3838

3939
TEST_CASE(

tiledb/api/c_api/context/context_api.cc

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
*/
3232

3333
#include "../config/config_api_internal.h"
34-
#include "../filesystem/filesystem_api_external.h"
3534
#include "context_api_external.h"
3635
#include "context_api_internal.h"
3736
#include "tiledb/api/c_api_support/c_api_support.h"

tiledb/api/c_api/dimension/dimension_api.cc

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
*
66
* The MIT License
77
*
8-
* @copyright Copyright (c) 2023 TileDB, Inc.
8+
* @copyright Copyright (c) 2023-2024 TileDB, Inc.
99
*
1010
* Permission is hereby granted, free of charge, to any person obtaining a copy
1111
* of this software and associated documentation files (the "Software"), to deal
@@ -32,6 +32,7 @@
3232

3333
#include "../../c_api_support/c_api_support.h"
3434
#include "../filter_list/filter_list_api_internal.h"
35+
#include "../string/string_api_internal.h"
3536
#include "dimension_api_external.h"
3637
#include "dimension_api_internal.h"
3738
#include "tiledb/api/c_api_support/exception_wrapper/exception_wrapper.h"
@@ -40,7 +41,7 @@
4041
namespace tiledb::api {
4142

4243
int32_t tiledb_dimension_alloc(
43-
tiledb_ctx_t* ctx,
44+
tiledb_ctx_handle_t* ctx,
4445
const char* name,
4546
tiledb_datatype_t type,
4647
const void* dim_domain,
@@ -138,12 +139,38 @@ int32_t tiledb_dimension_get_tile_extent(
138139

139140
int32_t tiledb_dimension_dump(const tiledb_dimension_t* dim, FILE* out) {
140141
ensure_dimension_is_valid(dim);
141-
dim->dump(out);
142+
ensure_cstream_handle_is_valid(out);
143+
144+
std::stringstream ss;
145+
ss << *dim;
146+
size_t r = fwrite(ss.str().c_str(), sizeof(char), ss.str().size(), out);
147+
if (r != ss.str().size()) {
148+
throw CAPIException(
149+
"Error writing dimension " + dim->name() + " to output stream");
150+
}
151+
152+
return TILEDB_OK;
153+
}
154+
155+
int32_t tiledb_dimension_dump_str(
156+
const tiledb_dimension_t* dim, tiledb_string_handle_t** out) {
157+
ensure_dimension_is_valid(dim);
158+
ensure_output_pointer_is_valid(out);
159+
160+
std::stringstream ss;
161+
ss << *dim;
162+
*out = tiledb_string_handle_t::make_handle(ss.str());
142163
return TILEDB_OK;
143164
}
144165

145166
} // namespace tiledb::api
146167

168+
std::ostream& operator<<(
169+
std::ostream& os, const tiledb_dimension_handle_t& dim) {
170+
os << *dim.dimension_;
171+
return os;
172+
}
173+
147174
using tiledb::api::api_entry_context;
148175

149176
CAPI_INTERFACE(
@@ -242,3 +269,12 @@ CAPI_INTERFACE(
242269
FILE* out) {
243270
return api_entry_context<tiledb::api::tiledb_dimension_dump>(ctx, dim, out);
244271
}
272+
273+
CAPI_INTERFACE(
274+
dimension_dump_str,
275+
tiledb_ctx_t* ctx,
276+
const tiledb_dimension_t* dimension,
277+
tiledb_string_handle_t** out) {
278+
return api_entry_context<tiledb::api::tiledb_dimension_dump_str>(
279+
ctx, dimension, out);
280+
}

tiledb/api/c_api/dimension/dimension_api_external.h

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
#include "../api_external_common.h"
3737
#include "../datatype/datatype_api_external.h"
3838
#include "../filter_list/filter_list_api_external.h"
39+
#include "../string/string_api_external.h"
3940

4041
// For the `FILE *` argument in `tiledb_dimension_dump`
4142
#include <stdio.h>
@@ -292,6 +293,31 @@ TILEDB_EXPORT int32_t tiledb_dimension_dump(
292293
const tiledb_dimension_t* dim,
293294
FILE* out) TILEDB_NOEXCEPT;
294295

296+
/**
297+
* Dumps the contents of a dimension in ASCII form to the selected string
298+
* output.
299+
*
300+
* The output string handle must be freed by the user after use.
301+
*
302+
* **Example:**
303+
*
304+
* @code{.c}
305+
* tiledb_string_t* tdb_string;
306+
* tiledb_dimension_dump_str(ctx, dimension, &tdb_string);
307+
* // Use the string
308+
* tiledb_string_free(&tdb_string);
309+
* @endcode
310+
*
311+
* @param ctx The TileDB context.
312+
* @param dimension The dimension.
313+
* @param out The output string.
314+
* @return `TILEDB_OK` for success and `TILEDB_ERR` for error.
315+
*/
316+
TILEDB_EXPORT int32_t tiledb_dimension_dump_str(
317+
tiledb_ctx_t* ctx,
318+
const tiledb_dimension_t* dimension,
319+
tiledb_string_t** out) TILEDB_NOEXCEPT;
320+
295321
#ifdef __cplusplus
296322
}
297323
#endif

0 commit comments

Comments
 (0)