Repository: pandas-dev/pandas
Pull request: ZeroDivisionError when groupby rank with method="dense" and pct=True
- [x] closes #23666 
- [x] tests added / passed
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry


Problem statement:
ZeroDivisionError when groupby rank with method="dense" and pct=True
When I tried to use groupby rank function with method="dense", pct=True options, I encountered the ZeroDivisionError. 

#### Code Sample, a copy-pastable example if possible

```python
import pandas as pd

df = pd.DataFrame({"A": [1, 1, 1, 2, 2, 2],
                   "B": [1, 1, 1, 1, 2, 2],
                   "C": [1, 2, 1, 1, 1, 2]})
df.groupby(["A", "B"])["C"].rank(method="dense", pct=True)
```
error:

```
Traceback (most recent call last):
  File "c:/Users/<user_name>/Documents/test.py", line 6, in <module>
    df.groupby(["A", "B"])["C"].rank(method="dense", pct=True)
  File "C:\Users\<user_name>\Anaconda3\lib\site-packages\pandas\core\groupby\groupby.py", line 1906, in rank
    na_option=na_option, pct=pct, axis=axis)
  File "C:\Users\<user_name>\Anaconda3\lib\site-packages\pandas\core\groupby\groupby.py", line 1025, in _cython_transform
    **kwargs)
  File "C:\Users\<user_name>\Anaconda3\lib\site-packages\pandas\core\groupby\groupby.py", line 2630, in transform
    return self._cython_operation('transform', values, how, axis, **kwargs)
  File "C:\Users\<user_name>\Anaconda3\lib\site-packages\pandas\core\groupby\groupby.py", line 2590, in _cython_operation
    **kwargs)
  File "C:\Users\<user_name>\Anaconda3\lib\site-packages\pandas\core\groupby\groupby.py", line 2664, in _transform
    transform_func(result, values, comp_ids, is_datetimelike, **kwargs)
  File "C:\Users\<user_name>\Anaconda3\lib\site-packages\pandas\core\groupby\groupby.py", line 2479, in wrapper
    return f(afunc, *args, **kwargs)
  File "C:\Users\<user_name>\Anaconda3\lib\site-packages\pandas\core\groupby\groupby.py", line 2431, in <lambda>
    kwargs.get('na_option', 'keep')
  File "pandas\_libs\groupby_helper.pxi", line 1292, in pandas._libs.groupby.group_rank_int64
ZeroDivisionError: float division
```

#### Problem description

I encountered ZeroDivisionError when I tried to use the groupby rank function. 

I can't find out exactly what a problem is.  But when I drop either method="dense" or pct=True option, the above code works.

If some elements in the above DataFrame are changed, this error disappear.  For example, the following code gives the expected output.

```python
df = pd.DataFrame({"A": [1, 1, 1, 2, 2, 2],
                   "B": [1, 1, 1, 1, 2, 2],
                   "C": [1, 2, 1, 0, 1, 2]}) # a little change in column C
df.groupby(["A", "B"])["C"].rank(method="dense", pct=True)
```

output:
```
0    0.5
1    1.0
2    0.5
3    1.0
4    0.5
5    1.0
Name: C, dtype: float64
```

#### Output of ``pd.show_versions()``

<details>

[paste the output of ``pd.show_versions()`` here below this line]

INSTALLED VERSIONS
------------------
commit: None
python: 3.6.6.final.0
python-bits: 64
OS: Windows
OS-release: 10
machine: AMD64
processor: Intel64 Family 6 Model 78 Stepping 3, GenuineIntel
byteorder: little
LC_ALL: None
LANG: None
LOCALE: None.None

pandas: 0.23.4
pytest: 3.5.1
pip: 10.0.1
setuptools: 39.1.0
Cython: 0.28.2
numpy: 1.14.5
scipy: 1.1.0
pyarrow: None
xarray: None
IPython: 6.4.0
sphinx: 1.7.4
patsy: 0.5.0
dateutil: 2.7.3
pytz: 2018.4
blosc: None
bottleneck: 1.2.1
tables: 3.4.3
numexpr: 2.6.5
feather: None
matplotlib: 3.0.0
openpyxl: 2.5.3
xlrd: 1.1.0
xlwt: 1.3.0
xlsxwriter: 1.0.4
lxml: 4.2.5
bs4: 4.6.0
html5lib: 1.0.1
sqlalchemy: 1.2.7
pymysql: None
psycopg2: None
jinja2: 2.10
s3fs: None
fastparquet: None
pandas_gbq: None
pandas_datareader: None

</details>

Hints from issue discussion:
I think this is caused due to groups of size 1. By removing (A, B) = (2, 1) group, the error goes away.

@WillAyd Do you mind if I tackle this? It's my first time contributing to pandas but I think I have a rough idea on how to fix the problem.
Go for it!
@WillAyd Sorry for bothering you, but I had a quick question related to bug fix contributions.

In my rough testing of the fix, I found another bug in groupby rank (checked to make sure that the bug also existed without my fix). Should I fix the bug in another commit in the same PR or should I create a new bug tracker and go from there?

The new bug isn't directly related to the current bug but arises from nearby code.
Typically best to create a separate issue and PR for tracking and review purposes

Commit message: Fixed pep8 in the tests file

Patch to review:
diff --git a/doc/source/whatsnew/v0.24.0.rst b/doc/source/whatsnew/v0.24.0.rst
index 4ff3cc728f7f7..6777dcc448355 100644
--- a/doc/source/whatsnew/v0.24.0.rst
+++ b/doc/source/whatsnew/v0.24.0.rst
@@ -1420,6 +1420,7 @@ Groupby/Resample/Rolling
 - Bug in :meth:`DataFrame.expanding` in which the ``axis`` argument was not being respected during aggregations (:issue:`23372`)
 - Bug in :meth:`pandas.core.groupby.DataFrameGroupBy.transform` which caused missing values when the input function can accept a :class:`DataFrame` but renames it (:issue:`23455`).
 - Bug in :func:`pandas.core.groupby.GroupBy.nth` where column order was not always preserved (:issue:`20760`)
+- Bug in :meth:`pandas.core.groupby.DataFrameGroupBy.rank` with ``method='dense'`` and ``pct=True`` when a group has only one member would raise a ZeroDivisionError (:issue:`23666`).
 
 Reshaping
 ^^^^^^^^^
diff --git a/pandas/_libs/groupby_helper.pxi.in b/pandas/_libs/groupby_helper.pxi.in
index 523d43f893aad..ffab9b69e1047 100644
--- a/pandas/_libs/groupby_helper.pxi.in
+++ b/pandas/_libs/groupby_helper.pxi.in
@@ -588,7 +588,16 @@ def group_rank_{{name}}(ndarray[float64_t, ndim=2] out,
                 if out[i, 0] != out[i, 0] or out[i, 0] == NAN:
                     out[i, 0] = NAN
                 else:
-                    out[i, 0] = out[i, 0] / grp_sizes[i, 0]
+                    with gil:
+                        try:
+                            out[i, 0] = out[i, 0] / grp_sizes[i, 0]
+                        except ZeroDivisionError:
+                            # BUG 23666 - tie-breaking for dense causes the
+                            # group sizes for groups of count 1 to become 0
+                            # causing a divide by zero error. The correct
+                            # behaviour should be divide by 1 a.k.a no change
+                            # to out[i, 0]
+                            pass
 {{endif}}
 {{endfor}}
 
diff --git a/pandas/tests/groupby/test_rank.py b/pandas/tests/groupby/test_rank.py
index e7e91572c56d1..b83fc423b1b96 100644
--- a/pandas/tests/groupby/test_rank.py
+++ b/pandas/tests/groupby/test_rank.py
@@ -290,3 +290,17 @@ def test_rank_empty_group():
     result = df.groupby(column).rank(pct=True)
     expected = DataFrame({"B": [0.5, np.nan, 1.0]})
     tm.assert_frame_equal(result, expected)
+
+
+def test_rank_zero_div():
+    # GH 23666
+    df = pd.DataFrame({
+        "A": [1, 1, 1, 2, 2, 2],
+        "B": [1, 1, 1, 1, 2, 2],
+        "C": [1, 2, 1, 1, 1, 2]
+    })
+
+    try:
+        df.groupby(["A", "B"])["C"].rank(pct=True, method="dense")
+    except ZeroDivisionError:
+        pytest.fail("Unexpected zero division error with groupby rank")


Code review comments:
diff_hunk: @@ -588,7 +588,16 @@ def group_rank_{{name}}(ndarray[float64_t, ndim=2] out,
                 if out[i, 0] != out[i, 0] or out[i, 0] == NAN:
                     out[i, 0] = NAN
                 else:
-                    out[i, 0] = out[i, 0] / grp_sizes[i, 0]
+                    with gil:
path: pandas/_libs/groupby_helper.pxi.in
text: @user1:
Instead of going back to the gil can't you just check if the denominator is zero?

diff_hunk: @@ -290,3 +290,17 @@ def test_rank_empty_group():
     result = df.groupby(column).rank(pct=True)
     expected = DataFrame({"B": [0.5, np.nan, 1.0]})
     tm.assert_frame_equal(result, expected)
+
+
+def test_rank_zero_div():
+    # GH 23666
+    df = pd.DataFrame({
+        "A": [1, 1, 1, 2, 2, 2],
+        "B": [1, 1, 1, 1, 2, 2],
+        "C": [1, 2, 1, 1, 1, 2]
+    })
+
+    try:
path: pandas/tests/groupby/test_rank.py
text: @user1:
You don't need a try except here - exceptions will be propagated naturally and cause a test failure

@user1:
Would rather just be explicit about the expected result and test for that

@author:
Thanks for the feedback! I've implemented the requested changes.
I appreciate the help.