BUG: Crosstab margins ignoring dropna
State: closed

To fix bug #12577 : Crosstab margins ignoring dropna


Labels:
name: Bug
name: Missing-data
description: np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate
name: Reshaping
description: Concat, Merge/Join, Stack/Unstack, Explode

Comments:
tests!

I did test several groups of data on my Mac and they worked out well.. Can I have some hints where could go wrong? Thanks!

you need to add tests to the codebase. `pandas/tools/tests/test_pivot.py`. see the [docs](http://pandas.pydata.org/pandas-docs/stable/contributing.html)

@OXPHOS Thanks for the PR!

Contributions to pandas require not only that code be modified, but also that you add tests that ensure the proper behavior. This helps ensure that when other people change code in the future, if they do something that breaks the code you've now written, when they run the test suite (which will include your tests) they'll catch the error. 

The section of the dogs that talks about it is here. http://pandas.pydata.org/pandas-docs/stable/contributing.html#test-driven-development-code-writing 

As a basic test, you can basically write out the case from the issue report. You should probably put around line 938 in `pandas/tools/tests/test_pivot.py`

Hi @nickeubank , thanks for your instruction and thanks @jreback !

I modified the codes and added tests.
For modification: truncated data pass to _add_margin if dropna == true in pivot.pivot_table()
For tests: added 3 tests in test_pivot.py. Yet I am not sure about the test style (i.e. class name). 

I tried to add another test case:
`df = DataFrame({'a':[1, np.nan, np.nan, np.nan, np.nan, 2], 'b':[np.nan, 3, 4, 4, 4, np.nan]})
print (crosstab(df.a,df.b, margins=True))`

All intersected entries are NaN. However, this case leads to a general fail (even without my modification) and to be honest I don't how what the expected result should be like. 

Error Information:

> ```
> raise ValueError('No objects to concatenate')
> ```
> 
> ValueError: No objects to concatenate

I assume we need try/except somewhere else upstream (but I am rushing for lunch..)

pls add a whatsnew entry 

@OXPHOS I think that the "general fail" you note where no objects intersect is actually behaving fine -- it's throwing an informative exception, and I can't think of another way it could be handled. 

Here are [docs](http://pandas.pydata.org/pandas-docs/stable/contributing.html#documenting-your-code) for adding a "whatsnew" entry -- basically, this is just a notification file that gives people a headsup about changes. I think this will be in version 0.18.1 (@jreback sound right?)

yep

... I am so sorry I ran flake8 as the first step for the tests but didn't go back to it right before commit or after modification. I know it took a long time to run through the test line. I'll do better this time.

Wow I passed! Thanks @nickeubank and @jreback guys for your help! I saw a conflict with base branch now. Anything else I should do? Thanks!

looks good. see my comment above. Have to handle another case here.

Okay so here's what I found:
- This is an independent issue. Apparently no previous tests have conditions `margins=True` and `dropna=False` at the same time.
- For now, I added `if` to `table.columns.names level` in `_add_margins` and avoided the error. I am not sure whether we need the same condition in `table.rows.names level`.
- But the problem raised from the `if not dropna: table = table.reindex_axis(m, axis=1)` in `pivot_table`, which erased all column names. I have been confused by the column names for a while. I was not sure whether the column names would show up on the side. Then it turns out that the show-up of column names completely depends on `dropna`, which is kinda weird. Apparently this is a problem for a while and I think it's worth fixing. However, I tried to play with `table = table.reindex_axis(m, axis=1)` but failed. I haven't come up with a good idea to solve the problem so far.

@OXPHOS ok let's do this.

going to merge your current PR.

Pls open an issue for the other case with a simple repro example (and cross reference to this PR).

Will open soon.