Skip to content

CLN: Replacing %s with .format in pandas/core/frame.py #20461

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

Merged
merged 5 commits into from
Apr 17, 2018

Conversation

AaronCritchley
Copy link
Contributor

@jreback jreback added the Clean label Mar 22, 2018
@jreback
Copy link
Contributor

jreback commented Mar 22, 2018

looks good. circle ci seems to be failing.

@codecov
Copy link

codecov bot commented Mar 28, 2018

Codecov Report

Merging #20461 into master will not change coverage.
The diff coverage is 88.23%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #20461   +/-   ##
=======================================
  Coverage   91.84%   91.84%           
=======================================
  Files         153      153           
  Lines       49295    49295           
=======================================
  Hits        45274    45274           
  Misses       4021     4021
Flag Coverage Δ
#multiple 90.23% <88.23%> (ø) ⬆️
#single 41.89% <0%> (ø) ⬆️
Impacted Files Coverage Δ
pandas/core/frame.py 97.16% <88.23%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4a34497...9931432. Read the comment docs.

@TomAugspurger
Copy link
Contributor

Merged in master and pushed. @AaronCritchley let us know if you notice that the CI all passes.

@@ -7575,4 +7589,4 @@ def _from_nested_dict(data):


def _put_str(s, space):
return ('%s' % s)[:space].ljust(space)
return '{s}'.format(s=s)[:space].ljust(space)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @TomAugspurger, thanks for merging in master, it looks like CI is failing here when given a unicode value for s:

s = 'σ', space = 5

    def _put_str(s, space):
>       return '{s}'.format(s=s)[:space].ljust(space)
E       UnicodeEncodeError: 'ascii' codec can't encode character u'\u03c3' in position 0: ordinal not in range(128)

I'm not sure on how Pandas approaches unicode and what the preferred way of solving is, I'm happy to implement something but would rather make sure it's in the Pandas style.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To clarify, not at my laptop atm but I have a feeling just making this:

u'{s}'.format(s=s)[:space].ljust(space)

would fix it, but it doesn't seem consistent with other .format occurrences, this could all be from a lack of understanding of how Pandas generally handles unicode though.

Copy link
Contributor

Choose a reason for hiding this comment

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

The u prefix is appropriate here.

@jreback jreback added this to the 0.23.0 milestone Apr 17, 2018
@jreback jreback merged commit 75295e1 into pandas-dev:master Apr 17, 2018
@jreback
Copy link
Contributor

jreback commented Apr 17, 2018

thanks!

@AaronCritchley AaronCritchley deleted the cln-16130-core-frame branch April 17, 2018 10:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants