Repository: pyjanitor-devs/pyjanitor (python)

Issue:
[DOC] bin_numeric parameters
# Brief Description of Fix


Currently, the docs for `bin_numeric` describes the parameters to be more restrictive than they actually are. According to the pyjanitor docs, `num_bins` is expected to be an Optional[int]:

<img width="665" alt="image" src="https://user-images.githubusercontent.com/30731072/147819958-bf32b65e-a6c4-476b-a2a1-bf936b5fbf0f.png">

However,
pyjanitor's `num_bins` is passed directly into the `bins` parameter of `pd.cut`, as in:
```python
# pyjanitor.bin_numeric src code snippet
df[str(to_column_name)] = pd.cut(df[str(from_column_name)], bins=num_bins)
```
But `bins` parameter of `pd.cut` also accepts list of values / intervalIndex, cf. [pandas docs](https://pandas.pydata.org/pandas-docs/version/1.3.4/reference/api/pandas.cut.html). Which means that at the moment, this is possible:

```python
df = pd.DataFrame(
    np.random.default_rng(42).integers(300, 800, size=(400, 2)),
    columns=['a', 'b'],
).reset_index()

(
    df
    .bin_numeric(from_column_name='a', to_column_name='binned_a', num_bins=[300, 450, 550, 650, 800])
    .head()
)
```
<img width="237" alt="image" src="https://user-images.githubusercontent.com/30731072/147819882-47f7c51e-7f36-4a35-8e3a-764027fec8e7.png">


I see 2 possibilities:
1. Rename `num_bins` to `bins`, deprecate the latter parameter name, and update pyjanitors' docs accordingly to inform users that bins need not necessarily be integer.
2. Keep `num_bins`; in which case, let's check and enforce that the user is passing in an integer (?)

I'm not fond of option 2; I use `pd.cut` with bins specified as a list of cut-values alot in my daily work, so I would prefer to keep this functionality open.

---

In the same vein, how about also allowing **kwargs to be passed in to `bin_numeric` (and pass it directly to `pd.cut` within) so it is more flexible? I don't see a good reason why we are only exposing the `bins`&`labels` parameters to the end user.

To be clear, what I'm proposing is:
```python
def bin_numeric(
    df: pd.DataFrame,
    from_column_name: Hashable,
    to_column_name: Hashable,
    bins: Union[int,...] = 5,  # I'm actually not sure how to annotate this properly at the moment...
    labels: Optional[str] = None,
    **kwargs,
) -> pd.DataFrame:
    if not labels:
        df[str(to_column_name)] = pd.cut(
            df[str(from_column_name)], bins=bins, **kwargs,
        )
    [...snip...]
```


# Relevant Context

<!-- Please put here, in bullet points, links to the relevant docs page. A few starting template points are available
to get you started. -->

- [Link to documentation page](https://pyjanitor-devs.github.io/pyjanitor/api/functions/#janitor.functions.bin_numeric.bin_numeric)
- [Link to exact file to be edited](https://github.com/pyjanitor-devs/pyjanitor/blob/dev/janitor/functions/bin_numeric.py)


Pull request description:
[ENH] Deprecate bin_numeric params and allow kwargs
# PR Description
This PR pertains to the [`bin_numeric`](https://pyjanitor-devs.github.io/pyjanitor/api/functions/#janitor.functions.bin_numeric.bin_numeric) function.

- Deprecate `num_bins` in favour of `bins` parameter.
- Remove `labels` parameter, and open `**kwargs` to the end user. As a result, we defer the `len(labels) == num_bins` check to pandas, don't do this check within pyjanitor.
- Add an additional test case to ensure `retbins` is not included in `kwargs`. Raise error if it is passed in. (Because retbins changes the output signature of `pd.cut`; we have to ensure only the pandas dataframe is returned.

**This PR resolves #969 .**

# PR Checklist

<!-- This checklist exists for newcomers who are not yet familiar with our requirements. If you are experienced with
the project, please feel free to delete this section. -->

Please ensure that you have done the following:

1. [X] PR in from a fork off your branch. Do not PR from `<your_username>`:`dev`, but rather from `<your_username>`:`<feature-branch_name>`.
<!-- Doing this helps us keep the commit history much cleaner than it would otherwise be. -->
2. [X] If you're not on the contributors list, add yourself to `AUTHORS.rst`.
<!-- We'd like to acknowledge your contributions! -->
3. [X] Add a line to `CHANGELOG.md` under the latest version header (i.e. the one that is "on deck") describing the contribution.
    - Do use some discretion here; if there are multiple PRs that are related, keep them in a single line.

# Automatic checks

There will be automatic checks run on the PR. These include:

- Building a preview of the docs on Netlify
- Automatically linting the code
- Making sure the code is documented
- Making sure that all tests are passed
- Making sure that code coverage doesn't go down.

# Relevant Reviewers

<!-- Finally, please tag relevant maintainers to review. -->

Please tag maintainers to review.

- @ericmjl


Patch:
diff --git a/AUTHORS.md b/AUTHORS.md
index f1c4fc1..e9d0bb4 100644
--- a/AUTHORS.md
+++ b/AUTHORS.md
@@ -101,3 +101,4 @@ Contributors
 - [@ericclessantostv](https://github.com/ericlessantostv) | [contributions](https://github.com/pyjanitor-devs/pyjanitor/issues?q=is%3Aclosed+mentions%3Aericclessantostv)
 - [@fireddd](https://github.com/fireddd) |  [contributions](https://github.com/pyjanitor-devs/pyjanitor/issues?q=is%3Aclosed+mentions%3Afireddd)
 - [@Zeroto521](https://github.com/Zeroto521) | [contributions](https://github.com/pyjanitor-devs/pyjanitor/pulls?utf8=%E2%9C%93&q=is%3Aclosed+mentions%3Zeroto521)
+- [@thatlittleboy](https://github.com/thatlittleboy) | [contributions](https://github.com/pyjanitor-devs/pyjanitor/issues?q=is%3Aclosed+mentions%3Athatlittleboy)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index eadc722..0a55586 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,8 @@
 
 ## [Unreleased]
 
+-   [ENH] Deprecate `num_bins` from `bin_numeric` in favour of `bins`, and allow generic `**kwargs` to be passed into `pd.cut`. Issue #969. @thatlittleboy
+
 ## [v0.22.0] - 2021-11-21
 
 -   [BUG] Fix conditional join issue for multiple conditions, where pd.eval fails to evaluate if numexpr is installed. #898 @samukweku
diff --git a/janitor/functions/bin_numeric.py b/janitor/functions/bin_numeric.py
index 5078f3e..6a035b8 100644
--- a/janitor/functions/bin_numeric.py
+++ b/janitor/functions/bin_numeric.py
@@ -1,60 +1,72 @@
-from typing import Hashable, Optional
+from typing import Optional, Union, Sequence
 import pandas_flavor as pf
 import pandas as pd
 
-from janitor.utils import deprecated_alias
+from janitor.utils import check, check_column, deprecated_alias
+
+
+ScalarSequence = Sequence[float]
 
 
 @pf.register_dataframe_method
-@deprecated_alias(from_column="from_column_name", to_column="to_column_name")
+@deprecated_alias(
+    from_column="from_column_name",
+    to_column="to_column_name",
+    num_bins="bins",
+)
 def bin_numeric(
     df: pd.DataFrame,
-    from_column_name: Hashable,
-    to_column_name: Hashable,
-    num_bins: int = 5,
-    labels: Optional[str] = None,
+    from_column_name: str,
+    to_column_name: str,
+    bins: Optional[Union[int, ScalarSequence, pd.IntervalIndex]] = 5,
+    **kwargs,
 ) -> pd.DataFrame:
     """
     Generate a new column that labels bins for a specified numeric column.
 
-    This method mutates the original DataFrame.
-
-    Makes use of pandas `cut()` function to bin data of one column,
-    generating a new column with the results.
-
-    ```python
-        import pandas as pd
-        import janitor
-        df = (
-            pd.DataFrame(...)
-            .bin_numeric(
-                from_column_name='col1',
-                to_column_name='col1_binned',
-                num_bins=3,
-                labels=['1-2', '3-4', '5-6']
-                )
-        )
-    ```
+    This method does not mutate the original DataFrame.
+
+    A wrapper around the pandas [`cut()`][pd_cut_docs] function to bin data of
+    one column, generating a new column with the results.
+
+    [pd_cut_docs]: https://pandas.pydata.org/docs/reference/api/pandas.cut.html
+
+    Example: Binning a numeric column with specific bin edges.
+
+        >>> import pandas as pd
+        >>> import janitor
+        >>> df = pd.DataFrame({"a": [3, 6, 9, 12, 15]})
+        >>> df.bin_numeric(
+        ...     from_column_name="a", to_column_name="a_binned",
+        ...     bins=[0, 5, 11, 15],
+        ... )
+            a  a_binned
+        0   3    (0, 5]
+        1   6   (5, 11]
+        2   9   (5, 11]
+        3  12  (11, 15]
+        4  15  (11, 15]
 
     :param df: A pandas DataFrame.
     :param from_column_name: The column whose data you want binned.
     :param to_column_name: The new column to be created with the binned data.
-    :param num_bins: The number of bins to be utilized.
-    :param labels: Optionally rename numeric bin ranges with labels. Number of
-        label names must match number of bins specified.
+    :param bins: The binning strategy to be utilized. Read the `pd.cut`
+        documentation for more details.
+    :param **kwargs: Additional kwargs to pass to `pd.cut`, except `retbins`.
     :return: A pandas DataFrame.
-    :raises ValueError: if number of labels do not match number of bins.
+    :raises ValueError: If `retbins` is passed in as a kwarg.
     """
-    if not labels:
-        df[str(to_column_name)] = pd.cut(
-            df[str(from_column_name)], bins=num_bins
-        )
-    else:
-        if not len(labels) == num_bins:
-            raise ValueError("Number of labels must match number of bins.")
-
-        df[str(to_column_name)] = pd.cut(
-            df[str(from_column_name)], bins=num_bins, labels=labels
-        )
+    if "retbins" in kwargs:
+        raise ValueError("`retbins` is not an acceptable keyword argument.")
+
+    check("from_column_name", from_column_name, [str])
+    check("to_column_name", to_column_name, [str])
+    check_column(df, from_column_name)
+
+    df = df.assign(
+        **{
+            to_column_name: pd.cut(df[from_column_name], bins=bins, **kwargs),
+        }
+    )
 
     return df
diff --git a/janitor/functions/groupby_agg.py b/janitor/functions/groupby_agg.py
index 18da20a..118b39e 100644
--- a/janitor/functions/groupby_agg.py
+++ b/janitor/functions/groupby_agg.py
@@ -35,7 +35,7 @@ def groupby_agg(
                                            agg='mean',
                                            agg_column_name="col1"
                                            new_column_name='col1_mean_by_group',
-                                           dropna = True/False)
+                                           dropna=True/False)
     ```
 
     Functional usage syntax:
@@ -56,6 +56,41 @@ def groupby_agg(
 
     Method chaining usage syntax:
 
+    ```python
+    df.groupby_agg(
+        by=['group', 'var1'],
+        agg='size',
+        agg_column_name='var1',
+        new_column_name='count',
+    )
+    ```
+
+           group  var1  count
+        0      1     1      4
+        1      1     1      4
+        2      1     1      4
+        3      1     1      4
+        4      1     2      1
+        5      2     1      1
+        6      2     2      3
+        7      2     2      3
+        8      2     2      3
+        9      2     3      1
+
+    If the data has null values,
+    you can include the null values by passing `False` to `dropna`;
+    this feature was introduced in Pandas 1.1:
+
+            name   type  num  nulls
+        0  black  chair    4    1.0
+        1  black  chair    5    1.0
+        2  black   sofa   12    NaN
+        3    red   sofa    4    NaN
+        4    red  plate    3    3.0
+
+    Let's get the count, including the null values,
+    grouping on `nulls` column:
+
     ```python
 
         df.groupby_agg(


Test patch:
diff --git a/tests/functions/test_bin_numeric.py b/tests/functions/test_bin_numeric.py
index 7df254b..4d688f7 100644
--- a/tests/functions/test_bin_numeric.py
+++ b/tests/functions/test_bin_numeric.py
@@ -23,13 +23,14 @@ def test_bin_numeric_expected_columns(df):
 
 @pytest.mark.functions
 @given(df=df_strategy())
-def test_bin_numeric_num_labels(df):
+def test_bin_numeric_kwargs_has_no_retbins(df):
 
     with pytest.raises(ValueError):
         labels = ["a", "b", "c", "d", "e"]
         df.bin_numeric(
             from_column_name="a",
             to_column_name="a_bin",
-            num_bins=6,
+            bins=5,
             labels=labels,
+            retbins=True,
         )