Repository: pandas-dev/pandas
Pull request: TST: added test for groupby when calling first, last, nth (GH29645)
- [x] closes #29645
- [x] tests added / passed
- [x] Ensure all linting tests pass, see [here](https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#code-standards) for how to run them

Provided test for `groupby` when callling `first`, `last`, `nth` on a `Series` with `None` or `np.nan`.

Tests and linter pass locally.


Problem statement:
Inconsistent behavior with None between "normal" pandas functions and groupby's last/first/nth
#### Code Sample, a copy-pastable example if possible

```python
> s1 = pd.Series(['x', 'y', None], index=[0, 0, 0])
> s1.isna()
0    False
0    False
0    True 
dtype: bool

> s2 = pd.Series(['x', 'y', np.nan], index=[0, 0, 0])
> s2.isna()
0    False
0    False
0    True 
dtype: bool

# But groupby doesn't!
> s1.groupby(level=0).last()
0    None
dtype: object

> s2.groupby(level=0).last()
0    y
dtype: object
# Expectation: these should output the same thing.
```
#### Problem description

The rest of pandas seems to treat None as the same as np.nan, so it is inconsistent and head-scratching why groupby's functions wouldn't.  The expected behavior would be for first/last/nth to treat None & `np.nan` like every other pandas function does.

#### Expected Output

```python
> s1.groupby(level=0).last()
0    y
dtype: object
```

#### Output of ``pd.show_versions()``

<details>

[paste the output of ``pd.show_versions()`` here below this line]
INSTALLED VERSIONS
------------------
commit: None
python: 3.6.7.final.0
python-bits: 64
OS: Linux
...

pandas: 0.24.1
...
</details>

Hints from issue discussion:
This looks to work on master now. Could use a test

```
In [12]: s1 = pd.Series(['x', 'y', None], index=[0, 0, 0])

In [13]: s1.isna()
Out[13]:
0    False
0    False
0     True
dtype: bool

In [14]: s2 = pd.Series(['x', 'y', np.nan], index=[0, 0, 0])

In [15]: s2.isna()
Out[15]:
0    False
0    False
0     True
dtype: bool

In [16]: s1.groupby(level=0).last()
Out[16]:
0    y
dtype: object

In [17]: s2.groupby(level=0).last()
Out[17]:
0    y
dtype: object
```

Commit message: TST: added test for groupby when calling first, last, nth (GH29645)

Patch to review:
diff --git a/pandas/tests/groupby/test_groupby.py b/pandas/tests/groupby/test_groupby.py
index a714abd461461..1e7f6592963e9 100644
--- a/pandas/tests/groupby/test_groupby.py
+++ b/pandas/tests/groupby/test_groupby.py
@@ -2419,3 +2419,27 @@ def test_rolling_wrong_param_min_period():
     result_error_msg = r"__init__\(\) got an unexpected keyword argument 'min_period'"
     with pytest.raises(TypeError, match=result_error_msg):
         test_df.groupby("name")["val"].rolling(window=2, min_period=1).sum()
+
+
+@pytest.mark.parametrize("method", ["first", "last", "nth"])
+def test_groupby_last_first_nth_with_none(method):
+    # GH29645
+    s1 = (
+        Series([None, "x", None, "y", None], index=[0, 0, 0, 0, 0])
+        .isna()
+        .groupby(level=0)
+    )
+    s2 = (
+        Series([np.nan, "x", np.nan, "y", np.nan], index=[0, 0, 0, 0, 0])
+        .isna()
+        .groupby(level=0)
+    )
+
+    if method == "nth":
+        s_none = getattr(s1, method)(2)
+        s_nan = getattr(s2, method)(2)
+    else:
+        s_none = getattr(s1, method)()
+        s_nan = getattr(s2, method)()
+
+    tm.assert_series_equal(s_none, s_nan)


Code review comments:
diff_hunk: @@ -2419,3 +2419,27 @@ def test_rolling_wrong_param_min_period():
     result_error_msg = r"__init__\(\) got an unexpected keyword argument 'min_period'"
     with pytest.raises(TypeError, match=result_error_msg):
         test_df.groupby("name")["val"].rolling(window=2, min_period=1).sum()
+
+
+@pytest.mark.parametrize("method", ["first", "last", "nth"])
+def test_groupby_last_first_nth_with_none(method):
path: pandas/tests/groupby/test_groupby.py
text: @user1:
Please define expected without applying a method and then parametrize over None and nan

@author:
I don't quite understand what you mean: the issue states that `None` and `np.nan` should lead to the same result when I `groupby` and apply a method. If I don't apply a method, then what's the point of the test? My reasoning was: I have two `Series` with `None` and `np.nan` and I have to test that grouping with a method gives the same output.

@user1:
We would want to test the expected output explicitly. If both yield the same, the result may be incorrect nevertheless

@author:
Ok, I think I get what you mean