Skip to content

to_html formatter not called for float values in a mixed-type column #25983

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Apr 5, 2019
Merged

to_html formatter not called for float values in a mixed-type column #25983

merged 5 commits into from
Apr 5, 2019

Conversation

@simonjayhawkins
Copy link
Member

@simonjayhawkins simonjayhawkins commented Apr 3, 2019 •
edited
Loading

can't shortcut completely without changing existing tested behaviour in a couple of tests.

for instance, the expected output for test_to_string_with_formatters in pandas\tests\io\formats\test_format.py has two spaces between float and object column, whereas shortcuting would only have one (the same as between the int and float columns)

  int  float    object
0 0x1 [ 1.0]  -(1, 2)-
1 0x2 [ 2.0]    -True-
2 0x3 [ 3.0]   -False-

if it's OK to change a couple of tests, we can remove this subtle leading_space bug in this PR.

@codecov
Copy link

codecov bot commented Apr 4, 2019

Codecov Report

Merging #25983 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #25983      +/-   ##
==========================================
- Coverage   91.85%   91.84%   -0.01%     
==========================================
  Files         175      175              
  Lines       52554    52556       +2     
==========================================
- Hits        48272    48271       -1     
- Misses       4282     4285       +3
Flag Coverage Δ
#multiple 90.4% <100%> (ø) ⬆️
#single 41.9% <60%> (-0.08%) ⬇️
Impacted Files Coverage Δ
pandas/io/formats/format.py 97.89% <100%> (ø) ⬆️
pandas/io/gbq.py 75% <0%> (-12.5%) ⬇️
pandas/core/frame.py 96.79% <0%> (-0.12%) ⬇️
pandas/util/testing.py 90.71% <0%> (+0.1%) ⬆️

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 1172d61...71e8b31. Read the comment docs.

@codecov
Copy link

codecov bot commented Apr 4, 2019 •
edited
Loading

Codecov Report

Merging #25983 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #25983      +/-   ##
==========================================
- Coverage   91.86%   91.85%   -0.01%     
==========================================
  Files         175      175              
  Lines       52547    52549       +2     
==========================================
- Hits        48271    48268       -3     
- Misses       4276     4281       +5
Flag Coverage Δ
#multiple 90.41% <100%> (ø) ⬆️
#single 41.9% <60%> (-0.07%) ⬇️
Impacted Files Coverage Δ
pandas/io/formats/format.py 97.89% <100%> (ø) ⬆️
pandas/io/gbq.py 75% <0%> (-12.5%) ⬇️
pandas/core/frame.py 96.79% <0%> (-0.12%) ⬇️
pandas/util/testing.py 90.62% <0%> (-0.11%) ⬇️

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 35156dc...4ef3149. Read the comment docs.

WillAyd
WillAyd requested changes Apr 4, 2019
View reviewed changes
pandas/tests/io/formats/test_to_html.py Outdated
def test_to_html_formatters_object_type():
# GH 13021
def f(x):
return x if type(x) is str else '${:,.0f}'.format(x)
Copy link
Member

@WillAyd WillAyd Apr 4, 2019

Choose a reason for hiding this comment

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

Use isinstance instead of type

pandas/tests/io/formats/test_to_html.py Outdated

df = pd.DataFrame([['a'], [0], [10.4], [3]], columns=['x'])
result = df.to_html(formatters=dict(x=f))
assert '$10' in result
Copy link
Member

@WillAyd WillAyd Apr 4, 2019

Choose a reason for hiding this comment

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

Can we make a better assertion here? I understand the original issue pertained to floats only but probably doesn't hurt to make an assertion around the other values here as well

@WillAyd WillAyd added IO HTML read_html, to_html, Styler.apply, Styler.applymap Output-Formatting __repr__ of pandas objects, to_string labels Apr 4, 2019
@jreback jreback added this to the 0.25.0 milestone Apr 4, 2019
jreback
jreback requested changes Apr 4, 2019
View reviewed changes
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.

looks fine, ex @WillAyd comments, this fully replicates the original issue?

@jreback jreback mentioned this pull request Apr 4, 2019
Merged
4 tasks
@simonjayhawkins
Copy link
Member Author

simonjayhawkins commented Apr 4, 2019

this fully replicates the original issue?

yes.

@jreback
Copy link
Contributor

jreback commented Apr 4, 2019

lgtm. ping on green.

@simonjayhawkins
Copy link
Member Author

simonjayhawkins commented Apr 4, 2019

green. but not all checks ran. needs a restart?

@jreback
Copy link
Contributor

jreback commented Apr 5, 2019

hmm yeah, merge master and push again.

@simonjayhawkins
Copy link
Member Author

simonjayhawkins commented Apr 5, 2019

gbq failure, otherwise good.

@jreback
Copy link
Contributor

jreback commented Apr 5, 2019

yeah working on fixing gbq now

@jreback jreback merged commit 77713d5 into pandas-dev:master Apr 5, 2019
@jreback
Copy link
Contributor

jreback commented Apr 5, 2019

thanks @simonjayhawkins

@simonjayhawkins simonjayhawkins mentioned this pull request Apr 5, 2019
Closed
jreback added a commit that referenced this pull request Apr 5, 2019
@jreback
Revert "to_html formatter not called for float values in a mixed-type…
fc5f292 
… column (#25983)"

This reverts commit 77713d5.
@jreback
Copy link
Contributor

jreback commented Apr 5, 2019

I reverted this: fc5f292 as CI is failing

@simonjayhawkins simonjayhawkins mentioned this pull request Apr 5, 2019
Closed
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@WillAyd WillAyd WillAyd requested changes

@jreback jreback jreback approved these changes

Assignees

No one assigned

Labels

IO HTML read_html, to_html, Styler.apply, Styler.applymap Output-Formatting __repr__ of pandas objects, to_string

Projects

None yet

Milestone

0.25.0

Development

Successfully merging this pull request may close these issues.

to_html formatter not called for float values in a mixed-type column

3 participants

@simonjayhawkins @jreback @WillAyd