Pivot nans fix
State: closed

this gets the ball rolling on #18030 as it is very easy to fix, but requires some decisions, mostly for structure and naming

the basic options as I see them:

0 - this PR as is (maybe some name change to this 26 character monster)

1 - Outright changing the `dropna=False` default behavior I thought would be a bad idea as it breaks backwards compatibility, but this solution is quite clunky.

2 - Change `dropna=True` behavior to satisfy the test added here, but again that is not backwards compatible, but seems reasonable.

Also, I've noticed that taking out the line
https://github.com/pandas-dev/pandas/blob/3f8c0c4f002b6bcc69e0e534cb0c502fbab00373/pandas/core/reshape/pivot.py#L177
doesn't break any of the tests, so there is that. (this helps 2)

- [x] closes #18030
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Reshaping
description: Concat, Merge/Join, Stack/Unstack, Explode
name: MultiIndex

Comments:
why can you not simply used the value in the ``observed`` keyword?
Made some changes to move towards solution 2, but this requires a change in default value to observed, and it breaks 2 tests referencing #21133 

those two tests basically require what this new one would like to eliminate (keeping records where values are all nan) which was possible with a new kwarg, but without it the combinations from observed/dropna cant work it out without modifying the old test
I'm sorry, is there any way forward with this?

Either past tests need to be modified and backwards compatibility broken, or the API needs to be bloated, or the whole thing abandoned.

Should I modify the tests that drop the nancols now?
@endremborza can you help summarize what behavior you are expecting from the various combinations of `dropna` / `observed` and how they differ from today? We typically don't just change the default values (note CI going red) so would have to think through options and best approach
simply put, I think ```observed=True``` with ```dropna=False``` should not drop combinations that are present, but filled with nans.

simplest example I can produce:
```python
df = pd.DataFrame(
    {"number": ["one", "two", "one", "three"], 
     "letter": ["a", "a" ,"b", "b"], 
     "y": [1, 2, np.nan, np.nan],
     "x": [1, 2, 3, 4]}
)

result = df.pivot_table(values="y",
                        columns=["number", "letter"],
                        index="x",
                        observed=True, dropna=False)
```

here result is

<table border="1" class="dataframe">
  <thead>
    <tr>
      <th>number</th>
      <th colspan="2" halign="left">one</th>
      <th colspan="2" halign="left">three</th>
      <th colspan="2" halign="left">two</th>
    </tr>
    <tr>
      <th>letter</th>
      <th>a</th>
      <th>b</th>
      <th>a</th>
      <th>b</th>
      <th>a</th>
      <th>b</th>
    </tr>
    <tr>
      <th>x</th>
      <th></th>
      <th></th>
      <th></th>
      <th></th>
      <th></th>
      <th></th>
    </tr>
  </thead>
  <tbody>
    <tr>
      <th>1</th>
      <td>1.0</td>
      <td>NaN</td>
      <td>NaN</td>
      <td>NaN</td>
      <td>NaN</td>
      <td>NaN</td>
    </tr>
    <tr>
      <th>2</th>
      <td>NaN</td>
      <td>NaN</td>
      <td>NaN</td>
      <td>NaN</td>
      <td>2.0</td>
      <td>NaN</td>
    </tr>
    <tr>
      <th>3</th>
      <td>NaN</td>
      <td>NaN</td>
      <td>NaN</td>
      <td>NaN</td>
      <td>NaN</td>
      <td>NaN</td>
    </tr>
    <tr>
      <th>4</th>
      <td>NaN</td>
      <td>NaN</td>
      <td>NaN</td>
      <td>NaN</td>
      <td>NaN</td>
      <td>NaN</td>
    </tr>
  </tbody>
</table>

I suppose it should be

<table border="1" class="dataframe">
  <thead>
    <tr>
      <th>number</th>
      <th colspan="2" halign="left">one</th>
      <th>three</th>
      <th>two</th>
    </tr>
    <tr>
      <th>letter</th>
      <th>a</th>
      <th>b</th>
      <th>b</th>
      <th>a</th>
    </tr>
    <tr>
      <th>x</th>
      <th></th>
      <th></th>
      <th></th>
      <th></th>
    </tr>
  </thead>
  <tbody>
    <tr>
      <th>1</th>
      <td>1.0</td>
      <td>NaN</td>
      <td>NaN</td>
      <td>NaN</td>
    </tr>
    <tr>
      <th>2</th>
      <td>NaN</td>
      <td>NaN</td>
      <td>NaN</td>
      <td>2.0</td>
    </tr>
    <tr>
      <th>3</th>
      <td>NaN</td>
      <td>NaN</td>
      <td>NaN</td>
      <td>NaN</td>
    </tr>
    <tr>
      <th>4</th>
      <td>NaN</td>
      <td>NaN</td>
      <td>NaN</td>
      <td>NaN</td>
    </tr>
  </tbody>
</table>

However, this breaks current tests as I (and the red CI) pointed out

> also, I just noticed that if I leave out the x column and the ```index="x"``` I get a weird attribute error, that I think should not happen, but that is another issue
Thanks for the PR but I think this is stale and has some disagreement over implementation. If interested in pursuing further let's discuss more in issue before coming back to code
@WillAyd I'm sorry but this has been an absolute disappointment. I like pandas and I believe in the idea of actively trying to fix your own issues with open source software, but the indecision and disinterest here has been painful.

I think I've been quite clear with how all this could be handled, but it seems nobody took the time and the effort to evaluate these options.

Calling it stale just claims that there was nothing wrong with the process, or the way this was handled, which is quite off-putting. 
@endremborza you never responded to any of my queries above

you need to engage in discussion otherwise we cannot move forwards