Skip to content
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

BUG: Fix to_latex with longtable (#17959) #17960

Merged
merged 3 commits into from
Dec 11, 2017

Conversation

syxolk
Copy link
Contributor

@syxolk syxolk commented Oct 23, 2017

pandas.DataFrame.to_latex(longtable=True) always contained \multicolumn{3}{r}{{Continued on next page}} regardless of the number of columns in the output. This is now changed to use the actual number of columns in the output.

@codecov
Copy link

codecov bot commented Oct 23, 2017

Codecov Report

Merging #17960 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #17960      +/-   ##
==========================================
- Coverage    91.6%   91.58%   -0.02%     
==========================================
  Files         153      153              
  Lines       51306    51306              
==========================================
- Hits        46998    46990       -8     
- Misses       4308     4316       +8
Flag Coverage Δ
#multiple 89.44% <100%> (ø) ⬆️
#single 40.72% <0%> (-0.12%) ⬇️
Impacted Files Coverage Δ
pandas/io/formats/format.py 96.17% <100%> (+0.14%) ⬆️
pandas/io/gbq.py 25% <0%> (-58.34%) ⬇️
pandas/core/frame.py 97.81% <0%> (-0.1%) ⬇️
pandas/core/indexes/datetimes.py 95.59% <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 ae74c2b...930471d. Read the comment docs.

@jreback jreback added the IO LaTeX to_latex label Oct 23, 2017
@gfyoung gfyoung added the Bug label Oct 23, 2017
@gfyoung
Copy link
Member

gfyoung commented Oct 24, 2017

@syxolk : You just need a whatsnew entry, and this should be good to go.

@syxolk syxolk force-pushed the fix-issue-17959-to-latex-longtable branch from b3eccbc to 7a71573 Compare October 25, 2017 00:36
@syxolk
Copy link
Contributor Author

syxolk commented Oct 25, 2017

I just added the whatsnew entry and squashed the commits.

@gfyoung
Copy link
Member

gfyoung commented Oct 25, 2017

@syxolk : That's great! @jreback PTAL

@@ -365,7 +365,7 @@ def test_to_latex_longtable(self, frame):
\midrule
\endhead
\midrule
\multicolumn{3}{r}{{Continued on next page}} \\
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 make sure we are testing for 0, 1, 2, 3 columns (you don't have to fully compare the output, just assert that the multicolumn directive is correct)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just added test cases for 1 and 3 columns. We already had a test case with 2 columns. I don't think tables with 0 columns make any sense. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

sure they do, an empty dataframe is possible

Copy link
Member

Choose a reason for hiding this comment

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

@jreback : The code has a deliberate short-circuit for an empty DataFrame, and this pattern will not be followed for 0 columns in fact. I added a couple of tests for that short-circuit.

@@ -998,6 +998,7 @@ I/O
- Bug in :meth:`DataFrame.to_html` in which there was no validation of the ``justify`` parameter (:issue:`17527`)
- Bug in :func:`HDFStore.select` when reading a contiguous mixed-data table featuring VLArray (:issue:`17021`)
- Bug in :func:`to_json` where several conditions (including objects with unprintable symbols, objects with deep recursion, overlong labels) caused segfaults instead of raising the appropriate exception (:issue:`14256`)
- Bug in :meth:`DataFrame.to_latex` with ``longtable=True`` where a latex multicolumn always spanned over three columns (:issue:`17959`)

Copy link
Contributor

Choose a reason for hiding this comment

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

move to 0.21.1

Copy link
Member

Choose a reason for hiding this comment

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

Done.

@jreback
Copy link
Contributor

jreback commented Nov 25, 2017

can you rebase & update

@jreback
Copy link
Contributor

jreback commented Dec 8, 2017

can you update?

@gfyoung
Copy link
Member

gfyoung commented Dec 8, 2017

@jreback : I can carry this to the finish line.

@gfyoung gfyoung self-assigned this Dec 8, 2017
@gfyoung gfyoung force-pushed the fix-issue-17959-to-latex-longtable branch from 23fc5d3 to 324b780 Compare December 8, 2017 05:45
@syxolk
Copy link
Contributor Author

syxolk commented Dec 10, 2017

@jreback : I can carry this to the finish line.

Thanks! Sorry, I was quite busy.

pandas.DataFrame.to_latex(longtable=True) always contained
\multicolumn{3}{r}{{Continued on next page}}
regardless of the number of columns in the output.
@gfyoung gfyoung force-pushed the fix-issue-17959-to-latex-longtable branch 2 times, most recently from 677a27d to 9b4d30e Compare December 11, 2017 03:29
* Move whatsnew to 0.21.1.txt
* Empty tests for empty DataFrame in to_latex
@gfyoung gfyoung force-pushed the fix-issue-17959-to-latex-longtable branch from 9b4d30e to 930471d Compare December 11, 2017 03:30
@gfyoung
Copy link
Member

gfyoung commented Dec 11, 2017

@jreback : All changes have been addressed. PTAL.

@jreback jreback added this to the 0.21.1 milestone Dec 11, 2017
@jreback jreback merged commit e909ea0 into pandas-dev:master Dec 11, 2017
@jreback
Copy link
Contributor

jreback commented Dec 11, 2017

thanks @syxolk @gfyoung

@gfyoung gfyoung removed their assignment Dec 11, 2017
TomAugspurger pushed a commit to TomAugspurger/pandas that referenced this pull request Dec 11, 2017
TomAugspurger pushed a commit that referenced this pull request Dec 12, 2017
gfyoung added a commit that referenced this pull request Dec 15, 2017
A vestige of gh-17960.  Should have been removed before merge.
yarikoptic added a commit to neurodebian/pandas that referenced this pull request Feb 22, 2018
Version 0.22.0

* tag 'v0.22.0': (777 commits)
  RLS: v0.22.0
  DOC: Fix min_count docstring (pandas-dev#19005)
  DOC: More 0.22.0 updates (pandas-dev#19002)
  TST: Remove pow test in expressions
  COMPAT: Avoid td.skip decorator
  DOC: 0.22.0 release docs (pandas-dev#18983)
  DOC: Include 0.22.0 whatsnew
  Breaking changes for sum / prod of empty / all-NA (pandas-dev#18921)
  ENH: Added a min_count keyword to stat funcs (pandas-dev#18876)
  RLS: v0.21.1
  DOC: Add date to whatsnew (pandas-dev#18740)
  DOC: Include 0.21.1 whatsnew
  DOC: Update relase notes (pandas-dev#18739)
  CFG: Ignore W503
  DOC: fix options table (pandas-dev#18730)
  ENH: support non default indexes in writing to Parquet (pandas-dev#18629)
  BUG: Fix to_latex with longtable (pandas-dev#17959) (pandas-dev#17960)
  Parquet: Add error message for no engine found (pandas-dev#18717)
  BUG: Categorical data fails to load from hdf when all columns are NaN (pandas-dev#18652)
  DOC: clean-up whatsnew file for 0.21.1 (pandas-dev#18690)
  ...
yarikoptic added a commit to neurodebian/pandas that referenced this pull request Feb 22, 2018
* releases: (777 commits)
  RLS: v0.22.0
  DOC: Fix min_count docstring (pandas-dev#19005)
  DOC: More 0.22.0 updates (pandas-dev#19002)
  TST: Remove pow test in expressions
  COMPAT: Avoid td.skip decorator
  DOC: 0.22.0 release docs (pandas-dev#18983)
  DOC: Include 0.22.0 whatsnew
  Breaking changes for sum / prod of empty / all-NA (pandas-dev#18921)
  ENH: Added a min_count keyword to stat funcs (pandas-dev#18876)
  RLS: v0.21.1
  DOC: Add date to whatsnew (pandas-dev#18740)
  DOC: Include 0.21.1 whatsnew
  DOC: Update relase notes (pandas-dev#18739)
  CFG: Ignore W503
  DOC: fix options table (pandas-dev#18730)
  ENH: support non default indexes in writing to Parquet (pandas-dev#18629)
  BUG: Fix to_latex with longtable (pandas-dev#17959) (pandas-dev#17960)
  Parquet: Add error message for no engine found (pandas-dev#18717)
  BUG: Categorical data fails to load from hdf when all columns are NaN (pandas-dev#18652)
  DOC: clean-up whatsnew file for 0.21.1 (pandas-dev#18690)
  ...
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.

to_latex wrong \multicolumn count
4 participants