BUG: fix to_latex() when using MultiIndex with NaN in (#14249) #19910
Conversation
|
@tomneep are you testing both issues in the issue? |
|
cc @toobaz if any comments |
|
@jreback yes, I only added one test but parametrized it to cover both of the issues |
Sorry for the delay. I think the patch works as intended, but I also think that replacing lev2 = self.frame.index.get_level_values(i).format()would be cleaner. |
| @@ -864,6 +864,7 @@ I/O | |||
| - Bug in :func:`read_csv` where missing values were not being handled properly when ``keep_default_na=False`` with dictionary ``na_values`` (:issue:`19227`) | |||
| - Bug in :func:`read_sas` where a file with 0 variables gave an ``AttributeError`` incorrectly. Now it gives an ``EmptyDataError`` (:issue:`18184`) | |||
| - Bug in :func:`DataFrame.to_latex()` where pairs of braces meant to serve as invisible placeholders were escaped (:issue:`18667`) | |||
| - Bug in :func:`DataFrame.to_latex()` where a ``NaN`` in a ``MultiIndex`` would cause an ``IndexError`` or incorrect output (:issue:`14249`) | |||
There was a problem hiding this comment.
where an all-NaN level in a MultiIndex
(I think)
There was a problem hiding this comment.
In the SO link in #14249 there is a case where NaNs are being silently replaced with other non-NaN values in the level and this fixes both that issue and the one actually reported
|
Hi @toobaz, Thanks for checking the patch, I wanted to keep the changes as minimal as possible as it is my first contribution. The tests all pass so if you prefer this approach (I certainly do) I can make a PR for that commit instead (maybe after a bit of tidying). |
|
Hi @jreback @toobaz The rewrite I mention in the comment above closes the issue this PR initially aimed to close as well as #19981, and an (as far as I can tell unreported) issue that The main thing I don't know, is whether I should create a new PR since the changes are now more substantial than this initial one, or if I can just keep using this one and commit the changes to this branch? |
|
can you rebase. we moved some files around. |
tomneep
force-pushed
the
to_latex_nan_fix
branch
from
5dbe1af to
2a72b60
Compare
Codecov Report
@@ Coverage Diff @@
## master #19910 +/- ##
==========================================
+ Coverage 91.77% 91.78% +<.01%
==========================================
Files 152 152
Lines 49185 49186 +1
==========================================
+ Hits 45140 45143 +3
+ Misses 4045 4043 -2
Continue to review full report at Codecov.
|
|
Ok, rebased. I also rebased my alternative branch that tidies this portion of the code up: c0daa15 My preference would be to merge that branch instead (in which case a couple more tests could be added) but let me know which you prefer. |
git diff upstream/master -u -- "*.py" | flake8 --diffThis attempt to close #14249 which actually reports two related issues (one in the issue one in the SO link) with to_latex() with MultiIndex when the MultiIndex contains a NaN. The output is now more consistent with what you'd get from to_csv() or to_html().