ENH add fill_value feature to pd.get_dummies
State: closed

 - [x] closes #15923
 - [x] tests added / passed
 - [x] passes ``git diff upstream/master --name-only -- '*.py' | flake8 --diff``
 - [x] whatsnew entry

Closes #15923

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

Comments:
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/15926?src=pr&el=h1) Report
> Merging [#15926](https://codecov.io/gh/pandas-dev/pandas/pull/15926?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/4502e82083f4e253630588665a4fc6002c4f32ed?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/15926/graphs/tree.svg?width=650&src=pr&height=150&token=eZ4WkYLtcO)](https://codecov.io/gh/pandas-dev/pandas/pull/15926?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #15926      +/-   ##
==========================================
+ Coverage   90.96%   90.97%   +<.01%     
==========================================
  Files         145      145              
  Lines       49557    49573      +16     
==========================================
+ Hits        45081    45097      +16     
  Misses       4476     4476
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `88.73% <100%> (ø)` | :arrow_up: |
| #single | `40.6% <0%> (-0.02%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/15926?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/reshape.py](https://codecov.io/gh/pandas-dev/pandas/pull/15926?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvcmVzaGFwZS5weQ==) | `99.3% <100%> (+0.02%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/15926?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/15926?src=pr&el=footer). Last update [4502e82...d761096](https://codecov.io/gh/pandas-dev/pandas/pull/15926?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/15926?src=pr&el=h1) Report
> Merging [#15926](https://codecov.io/gh/pandas-dev/pandas/pull/15926?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/6d90a436674a1e285d4577553b5cb75906a1bd27?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `92.18%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/15926/graphs/tree.svg?src=pr&token=eZ4WkYLtcO&width=650&height=150)](https://codecov.io/gh/pandas-dev/pandas/pull/15926?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #15926      +/-   ##
==========================================
+ Coverage   90.98%   90.99%   +<.01%     
==========================================
  Files         145      145              
  Lines       49556    49618      +62     
==========================================
+ Hits        45090    45150      +60     
- Misses       4466     4468       +2
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `88.76% <92.18%> (+0.01%)` | :arrow_up: |
| #single | `40.53% <9.37%> (-0.04%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/15926?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/reshape.py](https://codecov.io/gh/pandas-dev/pandas/pull/15926?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvcmVzaGFwZS5weQ==) | `99.3% <100%> (+0.02%)` | :arrow_up: |
| [pandas/types/cast.py](https://codecov.io/gh/pandas-dev/pandas/pull/15926?src=pr&el=tree#diff-cGFuZGFzL3R5cGVzL2Nhc3QucHk=) | `85.5% <81.48%> (-0.24%)` | :arrow_down: |
| [pandas/indexes/multi.py](https://codecov.io/gh/pandas-dev/pandas/pull/15926?src=pr&el=tree#diff-cGFuZGFzL2luZGV4ZXMvbXVsdGkucHk=) | `96.7% <0%> (ø)` | :arrow_up: |
| [pandas/io/parsers.py](https://codecov.io/gh/pandas-dev/pandas/pull/15926?src=pr&el=tree#diff-cGFuZGFzL2lvL3BhcnNlcnMucHk=) | `95.65% <0%> (+0.01%)` | :arrow_up: |
| [pandas/core/common.py](https://codecov.io/gh/pandas-dev/pandas/pull/15926?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvY29tbW9uLnB5) | `91.03% <0%> (+0.34%)` | :arrow_up: |
| [pandas/types/common.py](https://codecov.io/gh/pandas-dev/pandas/pull/15926?src=pr&el=tree#diff-cGFuZGFzL3R5cGVzL2NvbW1vbi5weQ==) | `94.48% <0%> (+0.83%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/15926?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/15926?src=pr&el=footer). Last update [6d90a43...f7ee8f5](https://codecov.io/gh/pandas-dev/pandas/pull/15926?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
I get hesitating. Let me know your decision before we do a lot work. 
Alright. Everything but what the option should be called is fixed, the linting too hopefully. :/
@jreback I'm happy to address these comments. Almost all of this testing code was copied from other parts of the test suite. Should all of that other code be cleaned up as well?

Also, are we set on the interface re: the other discussion on the issue thread?
> @jreback I'm happy to address these comments. Almost all of this testing code was copied from other parts of the test suite. Should all of that other code be cleaned up as well?

sure if you'd like. I haven't looked at *all* code (hahha). So for sure some non-conforming code exists. (things like cleaning up other code usually better in other PR though).


I am ok with the API. @TomAugspurger @jorisvandenbossche 
Could you summarize the final API from that other issue? Like one comment said, dropna is a bit weird since we aren't ever dropping rows or cols.

In a sense, the current behavior is like fillna=0. You do the dummy encoding, with NaN -> NaN, then fill those output NaNs with 0. Your proposal is to not fill them. That's all assuming dummy_na=False. You wouldn't want to allow fillna=None or nan.

So I could see either fillna=0, or the third value for dummy_na like now. Not a strong preference over those two. With fillna would there be weird interactions with dummy_na?

> On Apr 7, 2017, at 8:34 AM, Jeff Reback <notifications@github.com> wrote:
> 
> I am ok with the API. @TomAugspurger @jorisvandenbossche
> 
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or mute the thread.
> 

Yeah I think this is where the rub is and 100% agree! Right now the code is filling missing values by default using something like `fillna=0`. 

Here are the main considerations compiled in one spot:

1. I searched the rest of the code base and found no examples of a `fillna` keyword in any public API. In fact, pandas already provides a centralized way to fill missing values, so adding something that is different here doesn't make sense. There should be one, and hopefully only one, way to do things.
2. There is the further value checking that might need to be done with a `fillna` keyword.
3. As I stated in the other issue, `dropna=True` is weird since we are not dropping rows. In the other parts of pandas public API, I have only seen this keyword used in reduction-like operations (i.e., summing, taking an average, etc.) where you want the operation to ignore missing values. This usage would deviate from those parts of the API.
4. A further reason `dropna` is weird is that pandas also already provides a public API to drop data, namely the `dropna` method. Again, there should be one and only one way to do things.

So I think the best solution is to not fill missing values by default at all and let people use the already provided public API methods to do this. This keeps the API purely idiomatic. Further it would let people do things like drop rows with missing values before or after they apply the `get_dummies` function, making processing with these functions more robust to changes in the order of operations, more idempotent, etc.

Thus my suggested API is

1. Don't fill any NaN's in any situation.
2. The `dummy_na` keyword just adds the extra column with a flag if there are any NaNs for each row.

The current API with the PR as it stands now (i.e., with the `dummy_na='keep'` option) does this

1. By default (`dummy_na=False`), all NaNs are set to zero.
2. If `dummay_na=True`, fill NaNs to zero and add the column of missing value flags.
3. If `dummy_na='keep'`, don't add an extra missing value column and don't fill missing values to zero.

IMO, the simpler solution of just not filling missing values with zero by default makes the code easier to reason about, easier to maintain since there is not a maze of options, and fundamentally more consistent with the other pandas API conventions. 

OFC it is possible to go with the second solution above, but this adds more complication to the code, deviates from other parts of the pandas API, and leaves the code harder to reason about. 
@beckermr ``fillna`` is a function name, the one you are looking for is ``fill_value`` which is pretty common. e.g. see ``.unstack`` or ``.reindex``

so that actually might be a nice alternative here, though it would have to be ``fill_value=0`` to preserve the existing.
Thanks for the summary, will give feedback later.

Jeff is right, anywhere I used fillna it should have been fill_value. 

> On Apr 7, 2017, at 9:52 AM, Jeff Reback <notifications@github.com> wrote:
> 
> @beckermr fillna is a function name, the one you are looking for is fill_value which is pretty common. e.g. see .unstack or .reindex
> 
> so that actually might be a nice alternative here, though it would have to be fill_value=0 to preserve the existing.
> 
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or mute the thread.
> 

Ahhhhh @jreback. Thanks! 

Are there any other examples where `fill_value` is set to something besides `NaN` or `None`? 

I searched the code base and I cannot find any. So using `fill_value=0` would also deviate from the API of (the rest? most of the rest?) of the code.

We keep coming up with alternative solutions that generate their own conflicts with other parts of the pandas API. I think this all points to the fact that filling with zero by default is fundamentally in conflict with the rest of the API and so should be deprecated.
``fill_value=np.nan`` is generally the default.

So this *does* deviate from the API (as you have noted) 😄 

The question is it is worth breaking this (and it would be a bit break I think).

as @jorisvandenbossche noted, this is generally a 'final' function so filling is nice implicity.


See my response to this point:

> The current API makes too many assumptions. Right now, the current API asserts that if a label is missing, it is NONE of the labels you have. This cannot possibly be true in all cases.
>
> Instead, if NaNs were always propagated by default (even if an additional missing label column is added) then users would be required to reason about why their labels are missing and how they should fill nulls.
>
> For example, if labels are missing at random and the rest of the data is representative, then filling the missing one hot encoded rows with the fraction of each label would be a pretty good way to fill nulls.
>
> Of course pandas should not do this either since it should not be making assumptions about the input data.

Basically, pandas cannot promise that filling zeros is the right thing to do. Making as few assumptions about peoples' input data and letting them do things like

```python
data_final = pd.get_dummies(data).fillna(0)
```

is the preferred solution since it is explicit, idiomatic with respect to the rest of the API, and respectful to the user to give them the option to do what they like. 

Breaking this basic part of the API for just this one function for this one case using an assumption about the input data that we cannot possibly ensure is always true seems like an easy choice to me. Don't break the API. Filling and dropping missing values is a core use of pandas and this bit the of the API should be as pure as possible. 

Further, there are major version changes for a reason. Pandas regularly pushes backwards incompatible changes in a effort to improve the API and make the code better. My suggested solution is very much inline with this ongoing work.


I'm +1 on adding a `fill_value` with the default of 0.
I'm -1 on outright breaking API, by having the default be `None` or `np.nan` (not sure if this is being proposed or not).
I'm on the fence about adding `fill_value=0` with a deprecation warning that it will change to `None` in the future, probably -0 at this point.

Fortunately, we can defer discussion on what the default should be in the future.
Adding `fill_value=0` is backwards compatible, and let's us start a deprecation cycle if we decide to.

@beckermr just in case it's not clear, I think you're argument is completely valid. I just am on the fence about whether it's worth deprecating and changing in the future. While pandas has been aggressive about changing APIs, we're *trying* to become more stable. Are you OK with implementing `fill_value=0` for now, and kicking the deprecation stuff down the road?
I see your point of course, but I think the purity of the API on this core function is very important. 

I suspect that as the API is cleaned up, there will be less to change. In other words, once it is all internally consistent with clearly correct choices, no more of these changes that break stuff will need to be done in the first place. 

The other thing to be said is that supporting old features which have turned out to be problematic indefinitely is a bad practice too since it hampers the development of the code base as a whole.

I do get being queasy about making them. However, I don't think people will realistically ever move away from pandas, unless they move from python too. Given this, it is better to fix stuff sooner rather than later so that pandas can further mature into the core tool it is already.

I am happy with adding `fill_value=0`. I think we should also mark the deprecation of the zero default to a `NaN` default *now*. This will kick off the deprecation cycle and fix this as soon as realistically possible without really breaking stuff, which I am not in favor of.
@beckermr so just to be clear, we would leave ``dummy_na`` (as is) and add ``fill_value=None`` (with deprecation warning), where it would still fill with 0. ?

ok with deprecation actually (though *could* be just a ``DeprecationWarning``), rather than a full-fledged ``FutureWarning``.
Yes, this is close to my proposed solution. I would

1. Leave `dummy_na` as is.
2. Add `fill_value=0`. (It is weird to me for it to be set to None, but us to fill with zero anyways, so just be explicit and change the keyword later. Edit: You might have meant this. It was not clear to me. Sorry!)
3. Add either a DeprecationWarning or FutureWarning saying that `fill_value` will default to `NaN` or `None` in the future.
4. At some point, change the default and remove the warning.

I do not know the difference in how `DeprecationWarning` and `FutureWarning` are used so advice on this is much appreciated. 


yes you set ``fill_value=None`` to signal that someone hasn't passed *anything*, so you *know* to warn.
Ohhhh ok @jreback. Sounds good. 
@jreback @TomAugspurger Are we all in agreement then? 

I am happy to make the changes OFC.
I pushed the new version. Let me know what you think.

@jreback @TomAugspurger for viz
@jreback @TomAugspurger Concerning dtypes, we want to do the following?

1. If no NaNs and fill_value = np.nan, then return uint8.
2. If NaNs, figure out the minimal type given the type of fill_value and use that.


> @jreback @TomAugspurger Concerning dtypes, we want to do the following?
If no NaNs and fill_value = np.nan, then return uint8.
If NaNs, figure out the minimal type given the type of fill_value and use that.

yep, you can prob just default this to ``float32`` for 2) and ``uint8`` for 1) (the inference will give you ``float64``).  like

```
if is_integer(fill_value):
     dtype = np.uint8
elif is_float(fill_value):
     dtype = np.float32
else:
     raise ValueError(.....)
```
Cool. For signed ints (i.e., fill_value=-1), should I use int8 or int32?
> Cool. For signed ints (i.e., fill_value=-1), should I use int8 or int32?

good point, ``int8`` is fine
to be extra defensive you can do check if ``fill_value`` is in range of ``np.iinfo(np.int8).max`` (and .min), otherwise you can make it a ``int64`` (annoying but pls test as well).
Sounds good. A simple if block is not terribly difficult. :)
maybe better is to use ``pandas.types.cast.infer_dtype_from_scalar`` and add an argument there ``downcast=False`` (which you will set to ``True``), that for ``int/float`` can implement this logic.
I see what you mean, but I think this should be a separate issue that can be addressed in another PR. I am guessing the downcasting logic is repeated a lot in the code base? Probably better to look at the other uses and make sure that something centralized makes sense?