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

BUG: extra leading space in to_string when index=False #36094

Merged

Conversation

onshek
Copy link
Contributor

@onshek onshek commented Sep 3, 2020

This PR is mainly from #29670 contributed by @charlesdong1991, and I made a few changes on his work.
The code have passed all tests modified and added in test_format.py and test_to_latex.py.
Any comment is welcomed!

@pep8speaks
Copy link

pep8speaks commented Sep 3, 2020

Hello @onshek! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-09-06 02:50:26 UTC

Copy link
Member

@charlesdong1991 charlesdong1991 left a comment

Choose a reason for hiding this comment

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

@onshek nice! this probably needs to be moved to 1.2 whatsnew note?

@charlesdong1991 charlesdong1991 added Bug Output-Formatting __repr__ of pandas objects, to_string Regression Functionality that used to work in a prior pandas version labels Sep 3, 2020
@onshek
Copy link
Contributor Author

onshek commented Sep 3, 2020

@onshek nice! this probably needs to be moved to 1.2 whatsnew note?

done

@jreback jreback added this to the 1.2 milestone Sep 5, 2020
Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

lgtm. ping on green.

@charlesdong1991 if any comments.

Copy link
Member

@charlesdong1991 charlesdong1991 left a comment

Choose a reason for hiding this comment

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

@onshek since the default has changed from None to True, maybe nicer to update the docstring a bit as well, something like?

# current doc
leading_space : bool, optional

# suggested?
leading_space : bool, optional, default is True

@onshek
Copy link
Contributor Author

onshek commented Sep 5, 2020

@onshek since the default has changed from None to True, maybe nicer to update the docstring a bit as well, something like?

# current doc
leading_space : bool, optional

# suggested?
leading_space : bool, optional, default is True

Sure, working on it.

@onshek
Copy link
Contributor Author

onshek commented Sep 5, 2020

@jreback @charlesdong1991 Done. Thanks for your reviews :)

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

small comment, ping on green.

@@ -257,7 +255,7 @@ Conversion

Strings
^^^^^^^

- Bug in :meth:`Series.to_string` adding a leading space when ``index=False`` (:issue:`24980`)
Copy link
Contributor

Choose a reason for hiding this comment

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

can you also that this would affect DataFrame.to_latex and DataFrame.to_string

Copy link
Contributor Author

@onshek onshek Sep 5, 2020

Choose a reason for hiding this comment

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

can you also that this would affect DataFrame.to_latex and DataFrame.to_string

"also" waht? Do you mean give a mark here like:

Bug in :meth:Series.to_string adding a leading space when index=False and this would affect DataFrame.to_latex and DataFrame.to_string (:issue:24980)

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- Bug in :meth:`Series.to_string` adding a leading space when ``index=False`` (:issue:`24980`)
- Bug in :meth:`Series.to_string`, :meth:`DataFrame.to_string`, and :meth:`DataFrame.to_latex` adding a leading space when ``index=False`` (:issue:`24980`)

ping if you could make this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated :)

@jreback jreback merged commit aca77f7 into pandas-dev:master Sep 6, 2020
@jreback
Copy link
Contributor

jreback commented Sep 6, 2020

thanks @onshek very nice!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Output-Formatting __repr__ of pandas objects, to_string Regression Functionality that used to work in a prior pandas version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug on .to_string(index=False)
4 participants