BUG: Improve col_space in to_html to allow css length strings (#25941) #26012
Conversation
| result = result.split('tbody')[0] | ||
| hdrs = [x for x in result.split("\n") if re.search(r"<th[>\s]", x)] | ||
| for h in hdrs: | ||
| assert "min-width: 100%" in h |
There was a problem hiding this comment.
Can we parametrize this for a few length units and make a stronger assertion?
There was a problem hiding this comment.
Done, let me know if it is enough units and a strong enough assertion for you
There was a problem hiding this comment.
may also need a test with a multiIndex for the columns and also to_html(header=False).
Codecov Report
@@ Coverage Diff @@
## master #26012 +/- ##
==========================================
- Coverage 91.82% 91.81% -0.01%
==========================================
Files 175 175
Lines 52551 52553 +2
==========================================
- Hits 48256 48254 -2
- Misses 4295 4299 +4
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #26012 +/- ##
==========================================
+ Coverage 91.84% 91.97% +0.12%
==========================================
Files 175 175
Lines 52517 52374 -143
==========================================
- Hits 48233 48169 -64
+ Misses 4284 4205 -79
Continue to review full report at Codecov.
|
|
Hello @ArtificialQualia! 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 2019-04-28 19:40:53 UTC |
it makes sense to just be px, since the parameter is applied to all columns % makes less sense. |
pandas/core/frame.py
Outdated
| col_space='The minimum width of each column in CSS length ' | ||
| 'units. An int is assumed to be px units.\n\n' | ||
| ' .. versionadded:: 0.25.0\n' | ||
| ' Abillity to use str') |
There was a problem hiding this comment.
I think you need a blank line here. does this render properly when this doc-page is built (meaning w/o warning)?
There was a problem hiding this comment.
Didn't need an extra blank line, but I did need to indent the line under versionadded a little more to remove warnings. Should be all set now. Here is what it looks like rendered:
| assert "0.556" in notebook | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("unit", ['100px', '10%', '5em', 150]) |
There was a problem hiding this comment.
Per @simonjayhawkins does it make sense to allow percentages here?
|
Anything more needed on this PR? |
pandas/io/formats/html.py
Outdated
| """ | ||
| if header and self.fmt.col_space is not None: | ||
| if isinstance(self.fmt.col_space, int): | ||
| self.fmt.col_space = ('{colspace}px' |
There was a problem hiding this comment.
instead of actually changing this parameter, can you just use it directly; I would do this check instead in the initializer step
There was a problem hiding this comment.
moved to init as requested
|
lgtm. @WillAyd |
|
Thanks @ArtificialQualia ! |
5 participants
git diff upstream/master -u -- "*.py" | flake8 --diffAdds the ability to pass a string as the
col_spaceparameter into_html.For backwards compatibility and similarity for other
to_*functions, I kept in the ability to pass anint. Theintis automatically turned intopxunits (I chosepxbecause that is the default if you put a unit-less CSS length into chrome). If you want that removed and to always expect a string, let me know.