Skip to content
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 string formatting #52883

Closed
wants to merge 14 commits into from
Closed

Conversation

reddyrg1
Copy link
Contributor

@reddyrg1 reddyrg1 commented Apr 23, 2023

Copy link
Member

@phofl phofl left a comment

Choose a reason for hiding this comment

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

Please always add tests.

Whatsnew should go into 2.0.1

@reddyrg1
Copy link
Contributor Author

I have added the requested changes

@mroeschke mroeschke added the Output-Formatting __repr__ of pandas objects, to_string label Apr 24, 2023
reddyrg1 and others added 4 commits April 24, 2023 21:19
Co-authored-by: Matthew Roeschke <10647082+mroeschke@users.noreply.github.com>

Fixed regressions
~~~~~~~~~~~~~~~~~
- Fixed regression when :meth:`DataFrame.to_string` creates extra space for string dtypes (:issue:`52690`)
Copy link
Member

Choose a reason for hiding this comment

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

Could you merge in main and add this to the existing v2.0.2.rst file instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, should have done this

GH #52690

"""
# Expected Output
Copy link
Member

Choose a reason for hiding this comment

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

Could you remove these comments?

@@ -33,6 +34,7 @@ Other
~~~~~
- Raised a better error message when calling :func:`Series.dt.to_pydatetime` with :class:`ArrowDtype` with ``pyarrow.date32`` or ``pyarrow.date64`` type (:issue:`52812`)


Copy link
Member

Choose a reason for hiding this comment

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

Could you undo these new whitespaces?

Co-authored-by: Matthew Roeschke <10647082+mroeschke@users.noreply.github.com>
@@ -1392,7 +1392,10 @@ def _format(x):
fmt_values = []
for i, v in enumerate(vals):
if not is_float_type[i] and leading_space or self.formatter is not None:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if not is_float_type[i] and leading_space or self.formatter is not None:
if (not is_float_type[i] or self.formatter is not None) and leading_space:
fmt_values.append(f" {_format(v)}")

Comment on lines +1395 to +1398
if leading_space:
fmt_values.append(f" {_format(v)}")
else:
fmt_values.append(f"{_format(v)}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if leading_space:
fmt_values.append(f" {_format(v)}")
else:
fmt_values.append(f"{_format(v)}")

It seems this if/else is already handled by the else block below. I think a smaller change is sufficient. WDYT? @reddyrg1 @mroeschke @phofl

@github-actions
Copy link
Contributor

This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this.

@github-actions github-actions bot added the Stale label Jun 17, 2023
@mroeschke
Copy link
Member

Thanks for the pull request, but it appears to have gone stale. If interested in continuing, please merge in the main branch, address any review comments and/or failing tests, and we can reopen.

@mroeschke mroeschke closed this Jun 20, 2023
@yuanx749 yuanx749 mentioned this pull request Jun 26, 2023
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Output-Formatting __repr__ of pandas objects, to_string Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: DataFrame.to_string() creates extra space for string dtypes in pandas >=2.0
4 participants