Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

david1437
Copy link
Contributor

@david1437 david1437 commented May 21, 2025

Rationale for this change

#46403

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.

Copy link

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

Copy link
Member

@pitrou pitrou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

@pitrou pitrou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@david1437
Copy link
Contributor Author

@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
Copy link
Member

pitrou commented May 28, 2025

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
Copy link
Contributor Author

david1437 commented May 28, 2025 via email

@david1437
Copy link
Contributor Author

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

@david1437 david1437 requested a review from pitrou May 29, 2025 07:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants