DOC: combine with None at same position in both dataframes

[StefRe]

Location of the documentation

https://pandas.pydata.org/pandas-docs/stable/reference/api/pandas.DataFrame.combine.html

Documentation problem

The fourth example states:

However, if the same element in both dataframes is None, that None is preserved

In the code sample to illustrate this, however, None is not preserved:

df1 = pd.DataFrame({'A': [0, 0], 'B': [None, 4]})
df2 = pd.DataFrame({'A': [1, 1], 'B': [None, 3]})
df1.combine(df2, take_smaller, fill_value=-5)
    A    B
0  0 -5.0
1  0  3.0

Suggested fix for documentation

If the actual result is the intended behavior, this code sample should be removed. If None should indeed be preserved (but why should it?) then this should be filed as a bug.

[TomAugspurger]

I think the correct example to demonstrate the point is to not specify a fill_value.

In [5]: df1 = pd.DataFrame({'A': [0, 0], 'B': [None, 4]})
   ...: df2 = pd.DataFrame({'A': [1, 1], 'B': [None, 3]})
   ...: df1.combine(df2, take_smaller, )
Out[5]:
   A    B
0  0  NaN
1  0  3.0

So the lines

pandas/pandas/core/frame.py

Lines 5962 to 5965 in 6efb0b2

	>>> df1.combine(df2, take_smaller, fill_value=-5)
	A B
	0 0 -5.0
	1 0 3.0

should be update to remove fill_value and show the expected output.

[StefRe]

I'm afraid this doesn't sound logical to me for the following reason:

• Examples 1 and 2 show the regular case without any NaNs. So far so good.

• Example 3 shows how to use fill_value. OK, here one could first show what happens if one of the values is NaN without providing a fill_value (NaN propagates).

• Then comes our example 4 beginning with "However, if the same element in both dataframes is None...", so this is expected to be something in contrast to the case where just one element is None. If this case isn't special at all then why would we need a separate example for it?

I don't understand why anyone would ever expect fill_value to work differently for just one or for both values being NaN.
Sorry if I couldn't make my point clear as English is not my native language. In the end it's a real petty issue I just by chance came across.

[TomAugspurger]

Hmm I'm not sure.

Can you go through the git blame on that function to check recent changes? The implementation makes it pretty clear that the value is filled regardless.

[StefRe]

The behavior changed from rev 0.22.0 to 0.23.0, the changelog of 0.23.0 doesn't mention it. Will go through the changes between 0.22.0 and 0.23.0 now.

import pandas as pd
print(pd.__version__)
df1 = pd.DataFrame({'a': [1, 2, None]})
df2 = pd.DataFrame({'a': [0, None, None]})
print(f'combine {df1.a.values} with {df2.a.values}')
def take_smaller(s1, s2):
  print(f'this    : {s1.values}')
  print(f'other   : {s2.values}')
  return s1 if s1.sum() < s2.sum() else s2
print(f'combined: {df1.combine(df2, take_smaller, fill_value=-5).a.values}')

0.22.0
combine [ 1.  2. nan] with [ 0. nan nan]
this    : [ 1.  2. -5.]
other   : [ 0. -5. -5.]
combined: [ 0. -5. nan]

0.23.0
combine [ 1.  2. nan] with [ 0. nan nan]
this    : [ 1.  2. -5.]
other   : [ 0. -5. -5.]
combined: [ 0. -5. -5.]

[StefRe]

The line in question is

pandas/pandas/core/frame.py

Line 4119 in a00154d

arr[this_mask & other_mask] = np.nan

:

It was remove in this commit: 15e2221. The commit message just says "CLN: simplify combine_first"

The line was introduced here.

[StefRe]

@TomAugspurger I added the requested information. How to proceed? Should I make a PR for a documentation change?

[TomAugspurger]

I'm not sure, but c34ac74#diff-1e79abbbdd150d4771b91ea60a4e1cc7R1257 doesn't look like it intended to change the behavior.

[StefRe]

OK, so I understand this means to just leave it open for others to comment.