-
Notifications
You must be signed in to change notification settings - Fork 4k
GH-48608: [Python] Fix interpolate actual values in Message.__repr__ f-string #48656
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
Conversation
|
Thanks for opening a pull request! If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project. Then could you also rename the pull request title in the following format? or See also: |
|
@Aokizy2 looks great to me, please rename this PR to follow the recommended naming convention. |
|
|
|
@rustyconover OK,I've renamed this PR. |
rustyconover
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great to me.
|
Thanks finding this @rustyconover and for opening a PR @Aokizy2! |
|
I think the diff has too many formatting changes in it, can you have it not change so much existing code? |
|
sure,I'll change it |
|
Is it ok now? |
|
Thanks, that looks good @Aokizy2, will merge if CI passes. |
rok
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing this.
|
After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit 727106f. There weren't enough matching historic benchmark results to make a call on whether there were regressions. The full Conbench report has more details. |
Rationale for this change
Message.__repr__showed format placeholders ({self.type}) instead of actual values.What changes are included in this PR?
Add missing
fprefix to f-string inMessage.__repr__(pyarrow/lib.pxi).Are these changes tested?
Yes: added test verifying string representation shows actual values.
Are there any user-facing changes?
Yes (bug fix):
str(message)now shows values liketype: RecordBatchinstead of placeholders.related to issue #48608