Repository: pyjanitor-devs/pyjanitor (python)

Issue:
[BUG] Coalesce returns a Series, instead of a dataframe
# Brief Description

the [coalesce](https://pyjanitor.readthedocs.io/reference/janitor.functions/janitor.coalesce.html) function returns a Series, instead of a dataframe. Also, it may be more performant to use `bfill`, `ffill`, compared to the combination of `functools.reduce` and `combine_first`. Also, it would be nice to have a default value, in the event that there are still null values.

# System Information

- Operating system: /Linux/  
- Python version (required): 3.9

# Minimally Reproducible Code

```python
df=pd.DataFrame({'s1':[np.nan,np.nan,6,9,9],'s2':[np.nan,8,7,9,9]})

    s1   s2
0  NaN  NaN
1  NaN  8.0
2  6.0  7.0
3  9.0  9.0
4  9.0  9.0

df.coalesce(['s1', 's2'],'col3') # returns a Series, instead of assigning to the dataframe.

   col3
0   NaN
1   8.0
2   6.0
3   9.0
4   9.0

```



Pull request:
[ENH] Minor fixes
# PR Description

Please describe the changes proposed in the pull request:

-  `coalesce` returns the entire dataframe, along with a new column (if `target_column_name` is not None)
-  uses the `bfill(1).ffill(1)` syntax, which is faster than `reduce(combine.first)`
-  adds a `default_value` parameter, to fill any remaining nulls.
-  update `update_where` to use `pd.eval` for string-like conditions.


**This PR resolves #831 .**

<!-- As you go down the PR template, please feel free to delete sections that are irrelevant. -->

# 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/CHANGELOG.md b/CHANGELOG.md
index dae7408..a18b57a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,9 @@
 -   [ENH] Drop `fill_value` parameter from `complete`. Users can use `fillna` instead. @samukweku
 -   [BUG] Fix bug in `pivot_longer` with single level columns. @samukweku
 -   [BUG] Disable exchange rates API until we can find another one to hit. @ericmjl
+-   [ENH] Change `coalesce` to return columns; also use `bfill`, `ffill`,
+    which is faster than `combine_first` @samukweku
+-   [ENH] Use `eval` for string conditions in `update_where`. @samukweku
 
 ## [v0.20.14] - 2021-03-25
 
diff --git a/janitor/functions.py b/janitor/functions.py
index 4046568..b847042 100644
--- a/janitor/functions.py
+++ b/janitor/functions.py
@@ -28,7 +28,7 @@ import pandas as pd
 import pandas_flavor as pf
 from multipledispatch import dispatch
 from natsort import index_natsorted
-from pandas.api.types import union_categoricals, is_list_like
+from pandas.api.types import union_categoricals, is_list_like, is_bool_dtype
 from pandas.errors import OutOfBoundsDatetime
 from scipy.stats import mode
 from sklearn.preprocessing import LabelEncoder
@@ -929,17 +929,78 @@ def reorder_columns(
 
 
 @pf.register_dataframe_method
-@deprecated_alias(columns="column_names")
+@deprecated_alias(columns="column_names", new_column_name="target_column_name")
 def coalesce(
     df: pd.DataFrame,
     column_names: Iterable[Hashable],
-    new_column_name: Optional[str] = None,
-    delete_columns: bool = True,
+    target_column_name: Optional[str] = None,
+    default_value: Optional[Union[int, float, str]] = None,
 ) -> pd.DataFrame:
-    """Coalesce two or more columns of data in order of column names provided.
+    """
+    Coalesce two or more columns of data in order of column names provided.
+
+    This finds the first non-missing value at each position.
 
     This method does not mutate the original DataFrame.
 
+    Example:
+
+    .. code-block:: python
+
+        import pandas as pd
+        import janitor as jn
+
+        df = pd.DataFrame({"A": [1, 2, np.nan],
+                           "B": [np.nan, 10, np.nan],
+                           "C": [5, 10, 7]})
+
+             A     B   C
+        0  1.0   NaN   5
+        1  2.0  10.0  10
+        2  NaN   NaN   7
+
+        df.coalesce(column_names = ['A', 'B', 'C'],
+                    target_column_name = 'D')
+
+            A     B   C    D
+        0  1.0   NaN   5  1.0
+        1  2.0  10.0  10  2.0
+        2  NaN   NaN   7  7.0
+
+    If no target column is provided, then the first column is updated,
+    with the null values removed::
+
+        df.coalesce(column_names = ['A', 'B', 'C'])
+
+            A     B   C
+        0  1.0   NaN   5
+        1  2.0  10.0  10
+        2  7.0   NaN   7
+
+    If nulls remain, you can fill it with the `default_value`::
+
+        df = pd.DataFrame({'s1':[np.nan,np.nan,6,9,9],
+                           's2':[np.nan,8,7,9,9]})
+
+            s1   s2
+        0  NaN  NaN
+        1  NaN  8.0
+        2  6.0  7.0
+        3  9.0  9.0
+        4  9.0  9.0
+
+        df.coalesce(column_names = ['s1', 's2'],
+                    target_column_name = 's3',
+                    default_value = 0)
+
+            s1   s2   s3
+        0  NaN  NaN  0.0
+        1  NaN  8.0  8.0
+        2  6.0  7.0  6.0
+        3  9.0  9.0  9.0
+        4  9.0  9.0  9.0
+
+
     Functional usage syntax:
 
     .. code-block:: python
@@ -955,31 +1016,51 @@ def coalesce(
         df = pd.DataFrame(...).coalesce(['col1', 'col2'])
 
     The first example will create a new column called 'col3' with values from
-    'col2' inserted where values from 'col1' are NaN, then delete the original
-    columns. The second example will keep the name 'col1' in the new column.
+    'col2' inserted where values from 'col1' are NaN.
+    The second example will update the values of 'col1',
+    since it is the first column in `column_names`.
 
     This is more syntactic diabetes! For R users, this should look familiar to
     `dplyr`'s `coalesce` function; for Python users, the interface
     should be more intuitive than the :py:meth:`pandas.Series.combine_first`
-    method (which we're just using internally anyways).
+    method.
 
     :param df: A pandas DataFrame.
     :param column_names: A list of column names.
-    :param new_column_name: The new column name after combining.
-    :param delete_columns: Whether to delete the columns being coalesced
+    :param target_column_name: The new column name after combining.
+        If `None`, then the first column in `column_names` is updated,
+        with the Null values replaced.
+    :param default_value: A scalar to replace any remaining nulls
+        after coalescing.
     :returns: A pandas DataFrame with coalesced columns.
+    :raises ValueError: if length of `column_names` is less than 2.
     """
-    series = [df[c] for c in column_names]
 
-    def _coalesce(series1, series2):
-        return series1.combine_first(series2)
+    check("column_names", column_names, [list])
+    if target_column_name:
+        check("target_column_name", target_column_name, [str])
+    if default_value:
+        check("default_value", default_value, [int, float, str])
 
-    if delete_columns:
-        df = df.drop(columns=column_names)
-    if not new_column_name:
-        new_column_name = column_names[0]
-    df[new_column_name] = reduce(_coalesce, series)  # noqa: F821
-    return df
+    if not column_names:
+        return df
+
+    if len(column_names) < 2:
+        raise ValueError(
+            """
+            The number of columns to coalesce
+            should be a minimum of 2.
+            """
+        )
+    check_column(df, column_names)
+
+    if target_column_name is None:
+        target_column_name = column_names[0]
+    # bfill/ffill combo is faster than combine_first
+    outcome = df.filter(column_names).bfill(1).ffill(1).iloc[:, 0]
+    if outcome.hasnans and (default_value is not None):
+        outcome = outcome.fillna(default_value)
+    return df.assign(**{target_column_name: outcome})
 
 
 @pf.register_dataframe_method
@@ -3572,31 +3653,47 @@ def update_where(
     """
     Add multiple conditions to update a column in the dataframe.
 
-    This method mutates the original DataFrame.
+    This method does not mutate the original DataFrame.
 
     Example usage:
 
     .. code-block:: python
 
-        # The dataframe must be assigned to a variable first.
         data = {
             "a": [1, 2, 3, 4],
             "b": [5, 6, 7, 8],
             "c": [0, 0, 0, 0]
         }
         df = pd.DataFrame(data)
-        df = (
-            df
-            .update_where(
-                condition=("a > 2 and b < 8",
-                target_column_name='c',
-                target_val=10)
-            )
-        # a b  c
-        # 1 5  0
-        # 2 6  0
-        # 3 7 10
-        # 4 8  0
+
+           a  b  c
+        0  1  5  0
+        1  2  6  0
+        2  3  7  0
+        3  4  8  0
+
+        df.update_where(conditions = (df.a > 2) & (df.b < 8),
+                        target_column_name = 'c',
+                        target_val = 10)
+
+           a  b   c
+        0  1  5   0
+        1  2  6   0
+        2  3  7  10
+        3  4  8   0
+
+    `update_where` also supports pandas *query* style string expressions::
+
+        df.update_where(conditions = "a > 2 and b < 8",
+                        target_column_name = 'c',
+                        target_val = 10)
+
+           a  b   c
+        0  1  5   0
+        1  2  6   0
+        2  3  7  10
+        3  4  8   0
+
 
     :param df: The pandas DataFrame object.
     :param conditions: Conditions used to update a target column
@@ -3605,21 +3702,32 @@ def update_where(
         in dataframe, a new column will be created; note that entries that do
         not get set in the new column will be null.
     :param target_val: Value to be updated
-    :returns: An updated pandas DataFrame.
+    :returns: A pandas DataFrame.
     :raises IndexError: if ``conditions`` does not have the same length as
         ``df``.
     :raises TypeError: if ``conditions`` is not a pandas-compatible string
         query.
+    :raises ValueError: if ``conditions`` does not return a boolean array-like
+        data structure.
 
     .. # noqa: DAR402
     """
 
+    df = df.copy()
+
     # use query mode if a string expression is passed
     if isinstance(conditions, str):
-        conditions_index = df.query(conditions).index
-    else:
-        conditions_index = df.loc[conditions].index
-    df.loc[conditions_index, target_column_name] = target_val
+        conditions = df.eval(conditions)
+
+    if not is_bool_dtype(conditions):
+        raise ValueError(
+            """
+            Kindly ensure that `conditions` passed
+            evaluates to a Boolean dtype.
+            """
+        )
+
+    df.loc[conditions, target_column_name] = target_val
 
     return df
 
diff --git a/janitor/utils.py b/janitor/utils.py
index 13cd5f3..8273582 100644
--- a/janitor/utils.py
+++ b/janitor/utils.py
@@ -2019,7 +2019,6 @@ def _column_sel_dispatch(columns_to_select, df):  # noqa: F811
     else:
         stop = df_columns.get_loc(stop)
 
-    # allows for reverse selection - write test for this
     if start > stop:
         filtered_columns = df_columns[slice(stop, start + 1, step)][::-1]
     else:


Test patch:
diff --git a/tests/functions/test_coalesce.py b/tests/functions/test_coalesce.py
index d36f544..33f4d06 100644
--- a/tests/functions/test_coalesce.py
+++ b/tests/functions/test_coalesce.py
@@ -1,36 +1,75 @@
 import numpy as np
 import pandas as pd
 import pytest
+from pandas.testing import assert_frame_equal
 
 
-@pytest.mark.functions
-def test_coalesce_with_title():
-    df = pd.DataFrame(
+@pytest.fixture
+def df():
+    return pd.DataFrame(
         {"a": [1, np.nan, 3], "b": [2, 3, 1], "c": [2, np.nan, 9]}
-    ).coalesce(["a", "b", "c"], "d")
-    assert "a" not in df.columns
-    assert df.shape == (3, 1)
-    assert pd.isna(df).sum().sum() == 0
+    )
+
+
+def test_wrong_type_column_names(df):
+    """Raise Error if wrong type is provided for `column_names`."""
+    with pytest.raises(TypeError):
+        df.coalesce("a", "b")
+
+
+def test_wrong_type_target_column_name(df):
+    """Raise TypeError if wrong type is provided for `target_column_name`."""
+    with pytest.raises(TypeError):
+        df.coalesce(["a", "b"], target_column_name=["new_name"])
+
+
+def test_wrong_type_default_value(df):
+    """Raise TypeError if wrong type is provided for `default_value`."""
+    with pytest.raises(TypeError):
+        df.coalesce(
+            ["a", "b"], target_column_name="new_name", default_value=[1, 2, 3]
+        )
+
+
+def test_len_column_names_less_than_2(df):
+    """Raise Error if column_names length is less than 2."""
+    with pytest.raises(ValueError):
+        df.coalesce(["a"])
+
+
+def test_empty_column_names(df):
+    """Return dataframe if `column_names` is empty."""
+    assert_frame_equal(df.coalesce([]), df)
 
 
 @pytest.mark.functions
-def test_coalesce_without_title():
-    df = pd.DataFrame(
-        {"a": [1, np.nan, 3], "b": [2, 3, 1], "c": [2, np.nan, 9]}
-    ).coalesce(["a", "b", "c"])
-    assert "a" in df.columns
-    assert "b" not in df.columns
-    assert df.shape == (3, 1)
-    assert pd.isna(df).sum().sum() == 0
+def test_coalesce_with_title(df):
+    """Test output if `target_column_name` is provided."""
+    result = df.coalesce(["a", "b", "c"], "d")
+    expected_output = df.assign(
+        d=df["a"].combine_first(df["b"].combine_first(df["c"]))
+    )
+    assert_frame_equal(result, expected_output)
+
+
+@pytest.mark.functions
+def test_coalesce_without_title(df):
+    """Test output if `target_column_name` is not provided."""
+    result = df.coalesce(["a", "b", "c"])
+    expected_output = df.assign(
+        a=df["a"].combine_first(df["b"].combine_first(df["c"]))
+    )
+    assert_frame_equal(result, expected_output)
 
 
 @pytest.mark.functions
 def test_coalesce_without_delete():
+    """Test ouptut if nulls remain and `default_value` is provided."""
     df = pd.DataFrame(
-        {"a": [1, np.nan, 3], "b": [2, 3, 1], "c": [2, np.nan, 9]}
-    ).coalesce(["a", "b", "c"], delete_columns=False)
-    assert "a" in df.columns
-    assert "b" in df.columns
-    assert "c" in df.columns
-    assert df.shape == (3, 3)
-    assert pd.isna(df).sum().sum() == 1
+        {"s1": [np.nan, np.nan, 6, 9, 9], "s2": [np.nan, 8, 7, 9, 9]}
+    )
+    expected = df.assign(s3=df.s1.combine_first(df.s2).fillna(0))
+    result = df.coalesce(
+        ["s1", "s2"], target_column_name="s3", default_value=0
+    )
+    assert_frame_equal(result, expected)
diff --git a/tests/functions/test_update_where.py b/tests/functions/test_update_where.py
index 09e5402..167f571 100644
--- a/tests/functions/test_update_where.py
+++ b/tests/functions/test_update_where.py
@@ -21,11 +21,16 @@ def test_update_where(dataframe):
     )
 
 
-def test_update_where_query():
-    """Test that function works with pandas query-style string expression."""
-    df = pd.DataFrame(
+@pytest.fixture
+def df():
+    return pd.DataFrame(
         {"a": [1, 2, 3, 4], "b": [5, 6, 7, 8], "c": [0, 0, 0, 0]}
     )
+
+
+def test_update_where_query(df):
+    """Test that function works with pandas query-style string expression."""
+
     expected = pd.DataFrame(
         {"a": [1, 2, 3, 4], "b": [5, 6, 7, 8], "c": [0, 0, 10, 0]}
     )
@@ -34,3 +39,11 @@ def test_update_where_query():
     )
 
     assert_frame_equal(result, expected)
+
+
+def test_not_boolean_conditions(df):
+    """Raise Error if `conditions` is not a boolean type."""
+    with pytest.raises(ValueError):
+        df.update_where(
+            conditions=(df.a + 5), target_column_name="c", target_val=10,
+        )