GH-46403: [C++] Add support for limiting element size when printing data #46536

david1437 · 2025-05-21T19:56:33Z

Rationale for this change

What changes are included in this PR?

A new PrettyPrinter option is added to limit elements to 100 characters by default.

Are these changes tested?

Yes

Are there any user-facing changes?

Yes, the default length for outputted elements when stringifying them is now different so if a user was relying on ToString of an array with large elements that result may now be changed.

GitHub Issue: [C++] Avoid printing very large values #46403

github-actions · 2025-05-21T19:57:00Z

⚠️ GitHub issue #46403 has been automatically assigned in GitHub to PR creator.

pitrou

Thanks for this @david1437 . This looks good in general, some comments below.

cpp/src/arrow/pretty_print.cc

cpp/src/arrow/pretty_print_test.cc

…ting arrow structures

pitrou

There is a failure in the Python test suite (see CI results). Can you update the test for the new behavior?

cpp/src/arrow/pretty_print.cc

david1437 · 2025-05-27T20:51:14Z

@pitrou this one should be good as well now, only thing is the python test seems to have a different length omitted than I expected? Also it seems cant pass the pretty print options into the python version? not sure how the C++ bindings play into that

pitrou · 2025-05-28T14:56:29Z

Also it seems cant pass the pretty print options into the python version? not sure how the C++ bindings play into that

If you feel at ease with Cython, you could try to add the options to the Python bindings. Otherwise someone else could do it in another PR.

only thing is the python test seems to have a different length omitted than I expected?

Well, you can try to diagnose the issue?

david1437 · 2025-05-28T17:00:50Z

Not to sure on debugging python bindings calling in to c++, but I'll give it a try!

…

On Wed, May 28, 2025, 3:56 PM Antoine Pitrou ***@***.***> wrote: *pitrou* left a comment (apache/arrow#46536) <#46536 (comment)> Also it seems cant pass the pretty print options into the python version? not sure how the C++ bindings play into that If you feel at ease with Cython, you could try to add the options to the Python bindings. Otherwise someone else could do it in another PR. only thing is the python test seems to have a different length omitted than I expected? Well, you can try to diagnose the issue? — Reply to this email directly, view it on GitHub <#46536 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AGD5HPJOI65DVO5SXMAHN7D3AXFDHAVCNFSM6AAAAAB5UGEU5WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDSMJWGY2TMMZVGE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

david1437 · 2025-05-28T22:28:34Z

Added the python bindings and figured out the test issue with some GDB debugging. Apologies for not looking deeper into it earlier, I was a bit intimidated by the python bindings but it wasnt too bad after all. @pitrou let me know if any other adjustments are needed thanks

pitrou · 2025-06-02T12:41:38Z

python/pyarrow/array.pxi

@@ -1357,7 +1357,8 @@ cdef class Array(_PandasConvertible):
        return f'{type_format}\n{self}'

    def to_string(self, *, int indent=2, int top_level_indent=0, int window=10,
-                  int container_window=2, c_bool skip_new_lines=False):
+                  int container_window=2, c_bool skip_new_lines=False,
+                  int max_element_length=100):


I'm curious, is there a reason not to name this element_size_limit like in C++?

david1437 · 2025-06-02T13:02:35Z

No particular reason other then that it sounded a bit more clear to me what it's meaning was.

…

On Mon, Jun 2, 2025, 1:42 PM Antoine Pitrou ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In python/pyarrow/array.pxi <#46536 (comment)>: > @@ -1357,7 +1357,8 @@ cdef class Array(_PandasConvertible): return f'{type_format}\n{self}' def to_string(self, *, int indent=2, int top_level_indent=0, int window=10, - int container_window=2, c_bool skip_new_lines=False): + int container_window=2, c_bool skip_new_lines=False, + int max_element_length=100): I'm curious, is there a reason not to name this element_size_limit like in C++? — Reply to this email directly, view it on GitHub <#46536 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AGD5HPNYSXFOF3HE4HPVC2D3BRBBRAVCNFSM6AAAAAB5UGEU5WVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDQOBYGIZDONRSGM> . You are receiving this because you were mentioned.Message ID: ***@***.***>

pitrou · 2025-06-02T13:10:04Z

No particular reason other then that it sounded a bit more clear to me what it's meaning was.

Hmm, but then should we use the same name in C++?

david1437 · 2025-06-02T13:33:34Z

I can change it later tonight after work

…

On Mon, Jun 2, 2025, 2:10 PM Antoine Pitrou ***@***.***> wrote: *pitrou* left a comment (apache/arrow#46536) <#46536 (comment)> No particular reason other then that it sounded a bit more clear to me what it's meaning was. Hmm, but then should we use the same name in C++? — Reply to this email directly, view it on GitHub <#46536 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AGD5HPPIJYGQCD657FGGOKL3BREMHAVCNFSM6AAAAAB5UGEU5WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDSMZQGY2DINRQGU> . You are receiving this because you were mentioned.Message ID: ***@***.***>

david1437 · 2025-06-02T18:36:02Z

@pitrou thanks for all your help on this it should be good now

pitrou

Thanks @david1437 for the update. Looking fine in general, just a few more suggestions.

pitrou · 2025-06-03T07:04:02Z

cpp/src/arrow/pretty_print.cc

+  Write(data, options_.element_size_limit);
+}
+
+void PrettyPrinter::Write(std::string_view data, const uint64_t max_chars) {


I hadn't noticed this, but can this take a regular signed int, and the casting to size_t or uint64_t be done in the method body?

Suggested change

void PrettyPrinter::Write(std::string_view data, const uint64_t max_chars) {

void PrettyPrinter::Write(std::string_view data, const int max_chars) {

pitrou · 2025-06-03T07:05:04Z

python/pyarrow/array.pxi

+            Maximum length of a single element before it is truncated,
+            by default ``100``.


"Length" is ambiguous, it might be misunderstood as the array length. We could be more precise and say "Maximum number of characters" for example.

pitrou · 2025-06-03T07:06:17Z

python/pyarrow/array.pxi

@@ -1357,7 +1357,8 @@ cdef class Array(_PandasConvertible):
        return f'{type_format}\n{self}'

    def to_string(self, *, int indent=2, int top_level_indent=0, int window=10,
-                  int container_window=2, c_bool skip_new_lines=False):
+                  int container_window=2, c_bool skip_new_lines=False,
+                  int element_size_limit=100):


Can you perhaps add a unit test for passing a specific element_size_limit?

github-actions bot added Component: C++ awaiting review Awaiting review labels May 21, 2025

pitrou requested changes May 22, 2025

View reviewed changes

cpp/src/arrow/pretty_print.cc Outdated Show resolved Hide resolved

cpp/src/arrow/pretty_print_test.cc Outdated Show resolved Hide resolved

cpp/src/arrow/pretty_print_test.cc Show resolved Hide resolved

github-actions bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels May 22, 2025

david1437 added 2 commits May 22, 2025 20:12

apacheGH-46403: [C++] Add support for limiting element size when prin…

f4885a6

…ting arrow structures

fix: Change format when elements are truncated and add additional tests

588c744

david1437 force-pushed the GH-46403 branch from 32fa998 to 588c744 Compare May 22, 2025 19:13

david1437 requested a review from pitrou May 22, 2025 19:14

pitrou requested changes May 27, 2025

View reviewed changes

cpp/src/arrow/pretty_print.cc Outdated Show resolved Hide resolved

Use string_view::substr and fix python tests

5a899f2

david1437 requested review from AlenkaF, raulcd and rok as code owners May 27, 2025 18:21

github-actions bot added the Component: Python label May 27, 2025

david1437 added 3 commits May 27, 2025 19:46

fix lorem test

a8a0f9e

fix lorem test

84319ec

fix lorem test

61af9f8

fix lorem test

cbf787d

david1437 added 2 commits May 28, 2025 22:02

fix python test failure

714d135

fix add python bindings and test

16ba3e5

david1437 requested a review from pitrou May 29, 2025 07:37

pitrou reviewed Jun 2, 2025

View reviewed changes

pitrou changed the title ~~GH-46403: [C++] Add support for limiting element size when printing a…~~ GH-46403: [C++] Add support for limiting element size when printing data Jun 2, 2025

rename python binding to match c++ field name

a98f2b5

david1437 requested a review from pitrou June 2, 2025 18:36

pitrou requested changes Jun 3, 2025

View reviewed changes

review corrections

515dc13

david1437 requested a review from pitrou June 5, 2025 21:12

	void PrettyPrinter::Write(std::string_view data, const uint64_t max_chars) {
	void PrettyPrinter::Write(std::string_view data, const int max_chars) {

		Maximum length of a single element before it is truncated,
		by default ``100``.

GH-46403: [C++] Add support for limiting element size when printing data #46536

Are you sure you want to change the base?

GH-46403: [C++] Add support for limiting element size when printing data #46536

Conversation

david1437 commented May 21, 2025 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

github-actions bot commented May 21, 2025

Uh oh!

pitrou left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pitrou left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

david1437 commented May 27, 2025

Uh oh!

pitrou commented May 28, 2025

Uh oh!

david1437 commented May 28, 2025 via email

Uh oh!

david1437 commented May 28, 2025

Uh oh!

pitrou Jun 2, 2025

Choose a reason for hiding this comment

Uh oh!

david1437 commented Jun 2, 2025 via email

Uh oh!

pitrou commented Jun 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

david1437 commented Jun 2, 2025 via email

Uh oh!

david1437 commented Jun 2, 2025

Uh oh!

pitrou left a comment

Choose a reason for hiding this comment

Uh oh!

pitrou Jun 3, 2025

Choose a reason for hiding this comment

Uh oh!

pitrou Jun 3, 2025

Choose a reason for hiding this comment

Uh oh!

pitrou Jun 3, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

david1437 commented May 21, 2025 •

edited by github-actions bot

Loading

pitrou commented Jun 2, 2025 •

edited

Loading