-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
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
Fix DataFrame.to_string() justification #22437
Conversation
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.
I don't think we want to add tailing spaces to the output of .to_string()
, not sure what's the problem, can't it be fixed? I guess the leading space is ok.
DataFrame({'x': [0.1, 0.2, -0.3], 'y': [4, 5, 6]}), | ||
DataFrame({'x': [0.1, 0.2, -0.3], 'y': [0.4, 0.5, 0.6]}), | ||
DataFrame({'x': [0.1, 0.2, -0.3], 'y': [0.4, 0.5, -0.6]}), | ||
] |
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.
In cases like this is preferred to use pytest parametrization.
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.
Will fix if/when general direction of this PR is accepted.
I updated the original description to include current and proposed behavior. |
I just updated one of the test modules that was failing ( It seems a lot of things assume/depend on the As an aside, while fixing
outputs
|
#22505 is probably a better fix |
Closing in favor of #22505. Will reopen if needed. |
git diff upstream/master -u -- "*.py" | flake8 --diff
Fixes justification with
DataFrame.to_string(index=False)
, but breaks other tests :-(There are two side-effects of this PR, which will likely make it unacceptable, but wanted to get suggestions if possible.
Current output with
master
Output with this PR
IntArrayFormatter._format_strings
because the formatting changed from'{x: d}'
which adds a leading space for positive values, to'{x:d}'
. My impression is that this PR eliminates an arguably unnecessary extra space for positive integers, but I haven't looked at those tests in too much detail (eg I've never used to_latex).The tests that break and an example is shown below.
The root issue, I think, is that we have
'{x: d}'.format(x=x)
, but the current tests don't want the leading space for a simplepd.DataFrame({'x': [0, 1], 'y': [2, 3]}).to_string(index=False)
.I have another attempt at this issue, which explicitly expects the leading space, but may not be palatable to some...
Thanks.