BUG: to_html misses truncation indicators (...) when index=False #22786

simonjayhawkins · 2018-09-20T14:09:21Z

closes BUG: to_html misses truncation indicators (...) when index=False #15019
closes BUG: to_html misses truncation indicators (...) when index=False and max_cols < len(columns) #22783
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

pep8speaks · 2018-09-20T14:09:28Z

Hello @simonjayhawkins! Thanks for submitting the PR.

There are no PEP8 issues in the file pandas/io/formats/html.py !
There are no PEP8 issues in the file pandas/tests/io/formats/conftest.py !
There are no PEP8 issues in the file pandas/tests/io/formats/test_to_html.py !

pandas/tests/io/formats/conftest.py

WillAyd · 2018-09-20T14:51:58Z

@simonjayhawkins I know you have a few overlapping PRs open at this point - is this the one you think should go in first? Want to make sure I prioritize review accordingly

simonjayhawkins · 2018-09-20T14:59:55Z

@WillAyd : Yes Will. Although not printing the column names #22747 is a probably a simpler fix and could go first. I've not raised a PR for that yet, as there's been no affirmation it's an issue.

simonjayhawkins · 2018-09-20T15:31:59Z

@WillAyd : Will, the refactoring PR, #22726 is independent to keep style changes and bug fixes separate. So there is no reason that #22726 can't be progressed in parallel.

pandas/io/formats/html.py

pandas/tests/io/formats/conftest.py

pandas/tests/io/formats/test_to_html.py

codecov · 2018-09-21T09:30:46Z

Codecov Report

Merging #22786 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #22786      +/-   ##
==========================================
+ Coverage   92.24%   92.24%   +<.01%     
==========================================
  Files         161      161              
  Lines       51336    51338       +2     
==========================================
+ Hits        47357    47359       +2     
  Misses       3979     3979

Flag	Coverage Δ
#multiple	`90.64% <100%> (ø)`	⬆️
#single	`42.34% <0%> (-0.01%)`	⬇️

Impacted Files	Coverage Δ
pandas/io/formats/html.py	`93.18% <100%> (+0.04%)`	⬆️

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 c55057f...15bd352. Read the comment docs.

jreback

same comment as @WillAyd about the multiple PRs

jreback · 2018-09-23T12:26:53Z

pandas/io/formats/html.py

+            ncols = len(self.fmt.tr_frame.columns)
+            nrows = len(self.fmt.tr_frame)
+
+            row = []


can you give some comments on what is happening here, would not object to a self.write_row?

@jreback
The added code is copy and pasted from self._write_regular_rows()

But for the index=False case, the (row) index value is not added to row. The assignment to dot_col_ix and the value of nindex_levels in the self.write_tr call is also changed to account for this.

Currently the truncation tests in tests\io\formats\test_to_html.py are being skipped. Without test coverage the values of dot_col_ix and nindex_levels in self._write_regular_rows() can take on any value and pass the tests.

To avoid potential regression on the notebook display codepath (index=True), i have duplicated the code rather than make any changes to self._write_regular_rows()

Tests could be added but I wanted to avoid going out of scope on this PR.

The existing code for the index=False case was not in a function unlike the code for the index=True cases. However it was only 3 lines.

So I would agree that a self.write_row function would now be a good idea if it was unlikely that a future refactoring was not to use self._write_regular_rows() instead.

How should I proceed?

Tests could be added but I wanted to avoid going out of scope on this PR.

can you do a pre-cursor PR which locks down the tests first?

@simonjayhawkins i mean comments in the code

pandas/tests/io/formats/test_to_html.py

simonjayhawkins · 2018-09-23T22:39:49Z

@jreback : This PR is currently blocking #22655, hence the overlap. I've marked it as WIP for the moment.

jreback · 2018-10-18T16:19:30Z

can you merge master

…runcation-inicators

jreback · 2018-10-28T03:46:38Z

@WillAyd can you have another look here

WillAyd · 2018-10-28T22:41:14Z

pandas/io/formats/html.py

@@ -306,6 +306,8 @@ def _column_header():
            align = self.fmt.justify

            if truncate_h:
+                if self.fmt.index is False:


Instead of doing is False can you leverage the implicit truthiness here and do if not self.fmt.index? While not documented I believe index=0 and index=None are acceptable in this and other parsers, so would be ideal to handle those consistently with False

can you update this

jreback · 2018-11-11T23:38:19Z

pandas/io/formats/html.py

@@ -306,6 +306,8 @@ def _column_header():
            align = self.fmt.justify

            if truncate_h:
+                if self.fmt.index is False:


can you update this

jreback · 2018-11-11T23:38:44Z

pandas/io/formats/html.py

+            ncols = len(self.fmt.tr_frame.columns)
+            nrows = len(self.fmt.tr_frame)
+
+            row = []


@simonjayhawkins i mean comments in the code

jreback · 2018-11-11T23:39:13Z

pandas/tests/io/formats/test_to_html.py

@@ -1881,6 +1903,22 @@ def test_to_html_multiindex_max_cols(self):
        </table>""")
        assert result == expected

+    def test_to_html_truncation_index_false_max_rows(self, datapath):
+        # GH 15019
+        np.random.seed(seed=0)


do really need to set the seed?

jreback · 2018-11-11T23:39:29Z

pandas/tests/io/formats/test_to_html.py

+        # GH 15019
+        np.random.seed(seed=0)
+        df = pd.DataFrame(np.random.randn(5, 2))
+        result = df.to_html(max_rows=4, index=False)


can you parameterize over index=False and index=0

TomAugspurger · 2018-11-14T20:59:02Z

@WillAyd @jreback I think all your comments have been addressed, right?

jreback · 2018-11-14T21:25:30Z

pandas/tests/io/formats/test_to_html.py

+def expected_html(datapath, name):
+    """
+    Read HTML file from formats data directory.
+


might be nice to change some of the existing tests to use this function (future PR though)

PR #23747 to change existing tests to use this function.

WillAyd · 2018-11-15T05:55:50Z

@simonjayhawkins code lgtm. Can you resolve conflicts? Can merge thereafter on green

* upstream/master: BUG: to_html misses truncation indicators (...) when index=False (pandas-dev#22786) API/DEPR: replace "raise_conflict" with "errors" for df.update (pandas-dev#23657) BUG: Append DataFrame to Series with dateutil timezone (pandas-dev#23685) CLN/CI: Catch that stderr-warning! (pandas-dev#23706) ENH: Allow for join between two multi-index dataframe instances (pandas-dev#20356) Ensure Index._data is an ndarray (pandas-dev#23628) DOC: flake8-per-pr for windows users (pandas-dev#23707) DOC: Handle exceptions when computing contributors. (pandas-dev#23714) DOC: Validate space before colon docstring parameters pandas-dev#23483 (pandas-dev#23506) BUG-22984 Fix truncation of DataFrame representations (pandas-dev#22987)

…das-dev#22786)

simonjayhawkins added 2 commits September 20, 2018 14:44

add code comment

72b8011

add failing test cases

a7b2424

TomAugspurger reviewed Sep 20, 2018

View reviewed changes

pandas/tests/io/formats/conftest.py Outdated Show resolved Hide resolved

WillAyd requested changes Sep 20, 2018

View reviewed changes

pandas/io/formats/html.py Outdated Show resolved Hide resolved

pandas/tests/io/formats/conftest.py Outdated Show resolved Hide resolved

pandas/tests/io/formats/test_to_html.py Outdated Show resolved Hide resolved

simonjayhawkins added 2 commits September 21, 2018 03:04

fix failing tests

8a82df5

requested changes

f028b7d

jreback added Bug Output-Formatting __repr__ of pandas objects, to_string IO HTML read_html, to_html, Styler.apply, Styler.applymap labels Sep 23, 2018

jreback requested changes Sep 23, 2018

View reviewed changes

simonjayhawkins added 2 commits September 24, 2018 12:54

add docstring

4c1bca8

add whatsnew

2cd532e

simonjayhawkins mentioned this pull request Sep 29, 2018

TST: coverage for skipped tests in io/formats/test_to_html.py #22888

Merged

2 tasks

simonjayhawkins and others added 2 commits October 21, 2018 18:53

merge upstream/master

d8fe9b5

Merge remote-tracking branch 'upstream/master' into simonjayhawkins-t…

ecad6c8

…runcation-inicators

WillAyd requested changes Oct 28, 2018

View reviewed changes

Merge remote-tracking branch 'upstream/master' into truncation-inicators

46d171f

jreback requested changes Nov 11, 2018

View reviewed changes

simonjayhawkins added 2 commits November 12, 2018 22:27

requested changes

c6cb507

Merge remote-tracking branch 'upstream/master' into truncation-inicators

de99a65

jreback added this to the 0.24.0 milestone Nov 14, 2018

jreback approved these changes Nov 14, 2018

View reviewed changes

jreback reviewed Nov 14, 2018

View reviewed changes

WillAyd approved these changes Nov 15, 2018

View reviewed changes

resolve merge conflicts

15bd352

jreback merged commit 8af7637 into pandas-dev:master Nov 15, 2018

simonjayhawkins deleted the truncation-inicators branch November 15, 2018 14:18

simonjayhawkins mentioned this pull request Nov 16, 2018

TST/CLN: refactor tests\io\formats\test_to_html.py #23747

Merged

2 tasks

tm9k1 pushed a commit to tm9k1/pandas that referenced this pull request Nov 19, 2018

BUG: to_html misses truncation indicators (...) when index=False (pan…

b6969ba

…das-dev#22786)

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019

BUG: to_html misses truncation indicators (...) when index=False (pan…

901e0d3

…das-dev#22786)

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019

BUG: to_html misses truncation indicators (...) when index=False (pan…

18a4a1e

…das-dev#22786)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG: to_html misses truncation indicators (...) when index=False #22786

BUG: to_html misses truncation indicators (...) when index=False #22786

simonjayhawkins commented Sep 20, 2018 •

edited

Loading

pep8speaks commented Sep 20, 2018

WillAyd commented Sep 20, 2018

simonjayhawkins commented Sep 20, 2018

simonjayhawkins commented Sep 20, 2018

codecov bot commented Sep 21, 2018 •

edited

Loading

jreback left a comment

jreback Sep 23, 2018

simonjayhawkins Sep 23, 2018 •

edited

Loading

jreback Sep 25, 2018

jreback Nov 11, 2018

simonjayhawkins commented Sep 23, 2018 •

edited

Loading

jreback commented Oct 18, 2018

jreback commented Oct 28, 2018

WillAyd Oct 28, 2018

jreback Nov 11, 2018

jreback Nov 11, 2018

jreback Nov 11, 2018

jreback Nov 11, 2018

jreback Nov 11, 2018

TomAugspurger commented Nov 14, 2018

jreback Nov 14, 2018

simonjayhawkins Nov 17, 2018

WillAyd commented Nov 15, 2018

BUG: to_html misses truncation indicators (...) when index=False #22786

BUG: to_html misses truncation indicators (...) when index=False #22786

Conversation

simonjayhawkins commented Sep 20, 2018 • edited Loading

pep8speaks commented Sep 20, 2018

WillAyd commented Sep 20, 2018

simonjayhawkins commented Sep 20, 2018

simonjayhawkins commented Sep 20, 2018

codecov bot commented Sep 21, 2018 • edited Loading

Codecov Report

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simonjayhawkins Sep 23, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simonjayhawkins commented Sep 23, 2018 • edited Loading

jreback commented Oct 18, 2018

jreback commented Oct 28, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TomAugspurger commented Nov 14, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WillAyd commented Nov 15, 2018

simonjayhawkins commented Sep 20, 2018 •

edited

Loading

codecov bot commented Sep 21, 2018 •

edited

Loading

simonjayhawkins Sep 23, 2018 •

edited

Loading

simonjayhawkins commented Sep 23, 2018 •

edited

Loading