Repository: pandas-dev/pandas
Pull request: DOC: Clarify and add fill_value example in arithmetic ops
- [ ] closes #19653
- [ ] tests added / passed
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`


Problem statement:
DOC: Clarifiy fill_value behavior in arithmetic ops
When adding two DataFrames using `df1.add(df2)` one can use the `fill_value` parameter to fill in any NaNs that might come up. This parameter seems pretty broken:

```python
a = pd.DataFrame(np.ones((3, 2)))
b = pd.DataFrame(np.ones((4, 3)))
print(a + b)  # all good:
#      0    1   2
# 0  2.0  2.0 NaN
# 1  2.0  2.0 NaN
# 2  2.0  2.0 NaN
# 3  NaN  NaN NaN

print(a.add(b))  # all good
print(a.add(b, fill_value=0))  # broken
#      0    1    2
# 0  2.0  2.0  1.0
# 1  2.0  2.0  1.0
# 2  2.0  2.0  1.0
# 3  1.0  1.0  1.0
```
As you see, it filled the NaNs with `1.0`. Changing `fill_value=1` will fill everything with `2.0`. However, changing some of the values inside `b` leads to more peculiar results, and I couldn't really connect the dots and find some pattern.

This was observed on Python 3.6 on both Linux and Windows.

Thanks.

<details>
```
commit: None
python: 3.6.3.final.0
python-bits: 64
OS: Linux
OS-release: 3.10.0-514.el7.x86_64
machine: x86_64
processor: x86_64
byteorder: little
LC_ALL: None
LANG: en_US.UTF-8
LOCALE: en_US.UTF-8
pandas: 0.22.0
pytest: None
pip: 9.0.1
setuptools: 38.4.0
Cython: 0.27.3
numpy: 1.14.0
scipy: 1.0.0
pyarrow: None
xarray: None
IPython: 6.2.1
sphinx: 1.6.7
patsy: None
dateutil: 2.6.1
pytz: 2017.3
blosc: None
bottleneck: None
tables: None
numexpr: None
feather: None
matplotlib: 2.1.1
openpyxl: None
xlrd: None
xlwt: None
xlsxwriter: None
lxml: None
bs4: None
html5lib: 0.9999999
sqlalchemy: None
pymysql: None
psycopg2: None
jinja2: 2.10
s3fs: None
fastparquet: None
pandas_gbq: None
pandas_datareader: None
```
</details>

Hints from issue discussion:
pls read the documentation. This is doing an align then a fillna operation.

```
In [1]: a = pd.DataFrame(np.ones((3, 2))) * 2
   ...: b = pd.DataFrame(np.ones((4, 3)))
   ...: 

In [2]: a
Out[2]: 
     0    1
0  2.0  2.0
1  2.0  2.0
2  2.0  2.0

In [3]: b
Out[3]: 
     0    1    2
0  1.0  1.0  1.0
1  1.0  1.0  1.0
2  1.0  1.0  1.0
3  1.0  1.0  1.0

In [4]: ax, bx = a.align(b)

In [5]: ax
Out[5]: 
     0    1   2
0  2.0  2.0 NaN
1  2.0  2.0 NaN
2  2.0  2.0 NaN
3  NaN  NaN NaN

In [6]: bx
Out[6]: 
     0    1    2
0  1.0  1.0  1.0
1  1.0  1.0  1.0
2  1.0  1.0  1.0
3  1.0  1.0  1.0

In [7]: ax.fillna(0) + bx.fillna(0)
Out[7]: 
     0    1    2
0  3.0  3.0  1.0
1  3.0  3.0  1.0
2  3.0  3.0  1.0
3  1.0  1.0  1.0

In [8]: a.add(b,fill_value=0)
Out[8]: 
     0    1    2
0  3.0  3.0  1.0
1  3.0  3.0  1.0
2  3.0  3.0  1.0
3  1.0  1.0  1.0

```
Hi,

I wouldn't post a GitHub issue without reading the documentation first :)

The [documentation](https://pandas.pydata.org/pandas-docs/stable/generated/pandas.DataFrame.add.html) can be understood both ways, and actually I think that my "explanation" is more intuitive and straight-forward.

It currently simply states that: 
>fill_value: Fill missing (NaN) values with this value. If both DataFrame locations are missing, the result will be missing. 

It's _very_ unclear that it refers to two types of NaN values - (a) the ones that already exist in the data, (b) and the ones that are generated after the alignment step and before the addition step - but does not include the NaNs that are visible after a simple addition of the dataframes, which are the most relevant to the end user.

At the least I believe that the documentation should be edited, won't you think?
you are welcome to PR a doc update if u think it is not clear
Yep, I can see the point of confusion.

@HagaiHargil are you comfortable making a PR clarifying things?

https://github.com/pandas-dev/pandas/blob/a2771089d87afe8104765f12a07a6cd125e532ac/pandas/core/ops.py#L248-L259

and

https://github.com/pandas-dev/pandas/blob/a2771089d87afe8104765f12a07a6cd125e532ac/pandas/core/ops.py#L273-L284
I'd be happy to, I'm just having a hard time phrasing it. Here's a draft:

> fill_value : None or float value, default None 
     Fill existing missing (NaN) values, and any new element needed for successful array alignment with this value before computation. If data in both corresponding DataFrame locations is 
    missing, the result will be missing 

I'd also like to add an example, similar to the one in my initial post. Should it be added in both locations as well?
> Fill existing missing (NaN) values

That part is incorrect. *Existing* missing values will still be NA after the operation. Only newly-created missing values (created in the background by the `align` will be filled with `fill_value`). Everything else looks good though. So maybe something like

    Fill missing values created by alignment with this value before doing the operation.
    Existing missing values will not be filled by ``fill_value``.


```python
In [19]: a = pd.Series([1, 1, np.nan, np.nan], index=['a', 'b', 'c', 'd'])

In [20]: b = pd.Series([1, 1, np.nan, np.nan], index=['b', 'c', 'd', 'e'])

In [21]: a
Out[21]:
a    1.0
b    1.0
c    NaN
d    NaN
dtype: float64

In [22]: b
Out[22]:
b    1.0
c    1.0
d    NaN
e    NaN
dtype: float64

In [23]: a.add(b, fill_value=0)
Out[23]:
a    1.0
b    2.0
c    1.0
d    NaN
e    NaN
dtype: float64
```

> Should it be added in both locations as well?

Yep, both places (one using Series, one using DataFrame). Feel free to just put an example for `add` in there, even though it'll show up for e.g. `Series.sub`. I think people will get the idea.

It'd be good to include an example that has existing missing values and newly created missing values.
Umm, I might be misunderstanding you, but I think that it _does_ fill existing NA values:

```python
a = pd.DataFrame(np.ones((3, 2)))
b = pd.DataFrame(np.ones((4, 3)))

a.iloc[0, 0] = np.nan
a.add(b, fill_value=10)
#       0     1     2
# 0  11.0   2.0  11.0
# 1   2.0   2.0  11.0
# 2   2.0   2.0  11.0
# 3  11.0  11.0  11.0
```
I consider the NA value in `a[0, 0]` to be existing, in which case `fill_value` does turn it into a 10 before the addition.
I was apparently confused. It seems to fill when there are not NAs on both `left` and `right`. So valid + NA, both existing, will be filled.

```python
In [90]: a = pd.Series([1, 1, 1, np.nan], index=['a', 'b', 'c', 'd'])

In [91]: b = pd.Series([1, np.nan, 1, np.nan], index=['a', 'b', 'c_', 'd'])

In [92]: a
Out[92]:
a    1.0
b    1.0
c    1.0
d    NaN
dtype: float64

In [93]: b
Out[93]:
a     1.0
b     NaN
c_    1.0
d     NaN
dtype: float64

In [94]: a.add(b, fill_value=0)
Out[94]:
a     2.0
b     1.0
c     1.0
c_    1.0
d     NaN
dtype: float64
```

op                        | output
------------------------- | ------
valid + valid             | valid
valid + existing NA       | valid (valid + fill_value)
valid + new NA            | valid (valid + fill_value)
new NA + valid            | valid (fill_value + valid)
existing NA + existing NA | NA

That's a bit strange to me, but not worth changing at this point I think.

> DataFrame.add(other, axis=’columns’, level=None, fill_value=None)
>     Addition of dataframe and other, element-wise (binary operator add).
>     Equivalent to dataframe + other, but with support to substitute a fill_value for missing data _**in one
> of the inputs**_.

I think the document is clear.
An example would still be helpful.

On Mon, Feb 12, 2018 at 5:45 PM, ZhuBaohe <notifications@github.com> wrote:

> DataFrame.add(other, axis=’columns’, level=None, fill_value=None)
> Addition of dataframe and other, element-wise (binary operator add).
> Equivalent to dataframe + other, but with support to substitute a
> fill_value for missing data
> *in one of the inputs*.
>
> I think the document is clear.
>
> —
> You are receiving this because you modified the open/close state.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/issues/19653#issuecomment-365102059>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABQHIpGn7WYL-wta0z6HlLIAGbnp60Z5ks5tUMz9gaJpZM4SB21_>
> .
>

Commit message: Add fill_value example to DF

Patch to review:
diff --git a/pandas/core/ops.py b/pandas/core/ops.py
index 4c234ccb4dd47..a34b53be03625 100644
--- a/pandas/core/ops.py
+++ b/pandas/core/ops.py
@@ -255,8 +255,10 @@ def _get_frame_op_default_axis(name):
 ----------
 other : Series or scalar value
 fill_value : None or float value, default None (NaN)
-    Fill missing (NaN) values with this value. If both Series are
-    missing, the result will be missing
+    Fill existing missing (NaN) values, and any new element needed for 
+    successful array alignment, with this value before computation. 
+    If data in both corresponding DataFrame locations is missing 
+    the result will be missing
 level : int or name
     Broadcast across a level, matching Index values on the
     passed MultiIndex level
@@ -265,6 +267,18 @@ def _get_frame_op_default_axis(name):
 -------
 result : Series
 
+Examples
+--------
+>>> a = pd.Series([1, 1, 1, np.nan], index=['a', 'b', 'c', 'd'])
+>>> b = pd.Series([1, np.nan, 1, np.nan], index=['a', 'b', 'c_', 'd'])
+>>> a.add(b, fill_value=0)
+a     2.0
+b     1.0
+c     1.0
+c_    1.0
+d     NaN
+dtype: float64
+
 See also
 --------
 Series.{reverse}
@@ -280,8 +294,10 @@ def _get_frame_op_default_axis(name):
 axis : {0, 1, 'index', 'columns'}
     For Series input, axis to match Series index on
 fill_value : None or float value, default None
-    Fill missing (NaN) values with this value. If both DataFrame locations are
-    missing, the result will be missing
+    Fill existing missing (NaN) values, and any new element needed for 
+    successful array alignment, with this value before computation. 
+    If data in both corresponding DataFrame locations is missing 
+    the result will be missing
 level : int or name
     Broadcast across a level, matching Index values on the
     passed MultiIndex level
@@ -293,6 +309,18 @@ def _get_frame_op_default_axis(name):
 Returns
 -------
 result : DataFrame
+
+Examples
+--------
+>>> a = pd.Series([1, 1, 1, np.nan], index=['a', 'b', 'c', 'd'])
+>>> b = pd.Series([1, np.nan, 1, np.nan], index=['a', 'b', 'c_', 'd'])
+>>> a.add(b, fill_value=0)
+a     2.0
+b     1.0
+c     1.0
+c_    1.0
+d     NaN
+dtype: float64
 """
 
 _flex_doc_FRAME = """
@@ -307,8 +335,10 @@ def _get_frame_op_default_axis(name):
 axis : {{0, 1, 'index', 'columns'}}
     For Series input, axis to match Series index on
 fill_value : None or float value, default None
-    Fill missing (NaN) values with this value. If both DataFrame
-    locations are missing, the result will be missing
+    Fill existing missing (NaN) values, and any new element needed for 
+    successful array alignment, with this value before computation. 
+    If data in both corresponding DataFrame locations is missing 
+    the result will be missing
 level : int or name
     Broadcast across a level, matching Index values on the
     passed MultiIndex level
@@ -321,6 +351,18 @@ def _get_frame_op_default_axis(name):
 -------
 result : DataFrame
 
+Examples
+--------
+>>> a = pd.Series([1, 1, 1, np.nan], index=['a', 'b', 'c', 'd'])
+>>> b = pd.Series([1, np.nan, 1, np.nan], index=['a', 'b', 'c_', 'd'])
+>>> a.add(b, fill_value=0)
+      0
+a   2.0
+b   1.0
+c   1.0
+c_  1.0
+d   NaN
+
 See also
 --------
 DataFrame.{reverse}


Code review comments:
diff_hunk: @@ -293,6 +309,18 @@ def _get_frame_op_default_axis(name):
 Returns
 -------
 result : DataFrame
+
+Examples
+--------
+>>> a = pd.Series([1, 1, 1, np.nan], index=['a', 'b', 'c', 'd'])
path: pandas/core/ops.py
text: @user1:


Can you use a DataFrame for this example? It can just be a single-column DataFrame with these same values and index.

diff_hunk: @@ -255,8 +255,10 @@ def _get_frame_op_default_axis(name):
 ----------
 other : Series or scalar value
 fill_value : None or float value, default None (NaN)
-    Fill missing (NaN) values with this value. If both Series are
-    missing, the result will be missing
+    Fill existing missing (NaN) values, and any new element needed for 
+    successful array alignment, with this value before computation. 
+    If data in both corresponding DataFrame locations is missing 
path: pandas/core/ops.py
text: @user1:
This is for `Series`, so change `DataFrame` to `Series`.

@user1:
I would maybe rephrase it as

    Fill missing values with 'fill_value'. There are two sources of missing values

        * Missing values present either Series before the operation
        * Newly created missing values as a result of an alignment    

    In either case, missing values are not filled if both Series are missing after alignment.
    At least one value from either Series must be not NA for 'fill_value' to be used.


Though I'm sure if this is any clearer.

@author:
I have a problem with the first line - "Fill missing values with 'fill_value'". This is basically what bothered me in the first place, it doesn't _exactly_ fill missing value, in the naive way you would expect it to.

If you insist I'll gladly rephrase my current wording. In the mean time I'll fix the rest of your remarks.