Skip to content

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
Contributor

@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
Contributor

@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.

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
Contributor

@charlesdong1991 charlesdong1991 Sep 5, 2020

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