Latex bugs

[shangyian]

• closes to_latex() errors if index level name is not string #19981 and
  closes DataFrame.to_latex() outputs empty cells if MultiIndex names are empty strings #18669
• tests added / passed (see test_to_latex.py)
• passes git diff upstream/master -u -- "*.py" | flake8 --diff
• whatsnew entry

[shangyian]

I did some refactoring here and broke this method down into a couple of smaller ones. Ideally, pieces like _rebuild_multi_index() could also be a bit clearer, but maybe I'll tackle that while looking at some of the other Latex-related bugs.

[shangyian]

Also, after going through some of the other Latex-related bugs, I found this discussion about refactoring to_latex to use Styler. @TomAugspurger, do you think that still makes sense?

[jreback]

@shangyian so if going to refactor (great idea!). Would you mind doing a pre-cursor PR to split out to separate files, csv.py, html.py, latex.py (just the CSVFormatter and like classes). into separate module. Then should be much cleaner to re-factor this (e.g. you can make module level & static functions easily)

[jreback]

can you add doc-strings to added methods

[tomneep]

Hi @shangyian,
I came across this because I've also been looking at some to_latex bugs and have a PR open: #19910

As part of the discussion I also realised that what you've renamed _rebuild_multi_index() could be much cleaner. I had a go at this here https://github.com/tomneep/pandas/blob/to_latex_nan_fix_alternative/pandas/io/formats/format.py#L905 and think it really helps. I think it also addresses #19981 (as well as fixing a couple of other bugs). So maybe that is a potential option.

I have a question about one of your new tests, test_to_latex_multiindex_non_string, I'd expect the header to be 0 & 1 & \\ but in your test you have {} & 1 & \\. Am I missing something?

[shangyian]

@jreback - I did a preliminary PR to move the different formatters to separate files here: #20051. Let me know what you think.

[shangyian]

@tomneep - Good catch on the test - not sure why I thought that made sense! As for the multi-indexing portion of the method, yeah, it makes sense to rework that to be clearer. I didn't change much from the original code when I pulled it into _rebuild_multi_index.

[jreback]

ok let's rebase this and see where we are

[codecov]

Codecov Report

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

@@            Coverage Diff             @@
##           master   #20032      +/-   ##
==========================================
- Coverage   91.77%   91.77%   -0.01%     
==========================================
  Files         152      152              
  Lines       49181    49200      +19     
==========================================
+ Hits        45138    45155      +17     
- Misses       4043     4045       +2

Flag	Coverage Δ
#multiple	90.16% <100%> (-0.01%)	⬇️
#single	41.83% <0%> (-0.02%)	⬇️

Impacted Files	Coverage Δ
pandas/io/formats/latex.py	100% <100%> (ø)	⬆️
pandas/util/testing.py	83.74% <0%> (-0.21%)	⬇️

---

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 92c2910...32df285. Read the comment docs.

[shangyian]

Rebased and added some comments.

[shangyian]

Not sure if best to do this here, but we discussed combining get_level_lengths and _get_level_lengths in formats.style and I just did that now.

[pep8speaks]

Hello @shangyian! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on March 15, 2018 at 14:20 Hours UTC

[jreback]

can you rebase / fixup

[jreback]

closing as stale. if you'd like to continue pls ping.