Repository: pyjanitor-devs/pyjanitor
Pull request #1325 (2024-01-09 07:58:32)

[BUGFIX]`coalesce`
<!-- Thank you for your PR!

BEFORE YOU CONTINUE! Please add the appropriate three-letter abbreviation to your title.

The abbreviations can be:
- [DOC]: Documentation fixes.
- [ENH]: Code contributions and new features.
- [TST]: Test-related contributions.
- [INF]: Infrastructure-related contributions.

Also, do not forget to tag the relevant issue here as well.

Finally, as commits come in, don't forget to regularly rebase!
-->

# PR Description

Please describe the changes proposed in the pull request:

- ensure `get_index_labels` is called early in the code - column check, easy column selection ...

<!-- Doing so provides maintainers with context on what the PR is, and can help us more effectively review your PR. -->

<!-- Please also identify below which issue that has been raised that you are going to close. -->

**This PR improves coalesce.**

<!-- 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.md`.
<!-- We'd like to acknowledge your contributions! -->
3. [ ] 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


ericmjl: <!-- NETLIFY DEPLOY COMMENT GENERATED BY ACTIONS_NETLIFY -->
🚀 Deployed on https://deploy-preview-1325--pyjanitor.netlify.app
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/pyjanitor-devs/pyjanitor/pull/1325?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pyjanitor-devs) Report
> Merging [#1325](https://app.codecov.io/gh/pyjanitor-devs/pyjanitor/pull/1325?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pyjanitor-devs) (59a2936) into [dev](https://app.codecov.io/gh/pyjanitor-devs/pyjanitor/commit/92665c878eda9111fb3d238532c31a44de9c0157?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pyjanitor-devs) (92665c8) will **increase** coverage by `1.31%`.
> The diff coverage is `100.00%`.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##              dev    #1325      +/-   ##
==========================================
+ Coverage   93.22%   94.54%   +1.31%     
==========================================
  Files          78       78              
  Lines        4254     4256       +2     
==========================================
+ Hits         3966     4024      +58     
+ Misses        288      232      -56     
```

</details>
ericmjl: > @pyjanitor-devs/core-devs @ericmjl how do I check the minimum pandas version? in the tests, i can see pandas 1.3.5, whereas working in dev i see the minimum version is pandas 1.5.3

This is interesting, I need to check how the environments are built. Give me one moment, @samukweku!
ericmjl: @samukweku `environment-dev.yml` was pinned to 1.3.5. I'm not sure what the reason is, but we should release the pin and let it float with the ecosystem. I'm going to make a push.
ericmjl: @samukweku I'm not sure what's happening after releasing the version pin, but `pivot_wider`'s docstring examples appear to be failing. I made sure pytest skipped those tests, since we are going to be deprecating that function anyways.
ericmjl: Since I pushed to the PR, that means I've accepted the contents of it :smile:. Officially approving, and now going to merge! Thank you, @samukweku!

Patch:
diff --git a/environment-dev.yml b/environment-dev.yml
index 3c59895..1f8e48e 100644
--- a/environment-dev.yml
+++ b/environment-dev.yml
@@ -29,7 +29,7 @@ dependencies:
   - openpyxl
   - pandas-flavor
   - pandas-vet
-  - pandas=1.3.5
+  - pandas>=2.0
   - pip
   - pipreqs
   - pip-tools
diff --git a/janitor/functions/coalesce.py b/janitor/functions/coalesce.py
index 9178ecc..f7d4c52 100644
--- a/janitor/functions/coalesce.py
+++ b/janitor/functions/coalesce.py
@@ -4,7 +4,7 @@ from typing import Any, Optional, Union
 import pandas as pd
 import pandas_flavor as pf
 
-from janitor.functions.utils import get_index_labels
+from janitor.functions.utils import _select_index
 from janitor.utils import check, deprecated_alias
 
 
@@ -29,6 +29,9 @@ def coalesce(
 
     This method does not mutate the original DataFrame.
 
+    The [`select`][janitor.functions.select.select] syntax
+    can be used in `column_names`.
+
     Examples:
         Use `coalesce` with 3 columns, "a", "b" and "c".
 
@@ -97,24 +100,34 @@ def coalesce(
     if not column_names:
         return df
 
-    if len(column_names) < 2:
+    indexers = _select_index([*column_names], df, axis="columns")
+
+    if len(indexers) < 2:
         raise ValueError(
             "The number of columns to coalesce should be a minimum of 2."
         )
 
-    column_names = get_index_labels([*column_names], df, axis="columns")
-
     if target_column_name:
         check("target_column_name", target_column_name, [str])
 
     if default_value:
         check("default_value", default_value, [int, float, str])
 
-    if target_column_name is None:
-        target_column_name = column_names[0]
+    df = df.copy()
+
+    outcome = df.iloc[:, indexers[0]]
+
+    for num in range(1, len(indexers)):
+        position = indexers[num]
+        replacement = df.iloc[:, position]
+        outcome = outcome.fillna(replacement)
 
-    outcome = df.loc(axis=1)[column_names].bfill(axis="columns").iloc[:, 0]
     if outcome.hasnans and (default_value is not None):
         outcome = outcome.fillna(default_value)
 
-    return df.assign(**{target_column_name: outcome})
+    if target_column_name is None:
+        df.iloc[:, indexers[0]] = outcome
+    else:
+        df[target_column_name] = outcome
+
+    return df
diff --git a/janitor/functions/pivot.py b/janitor/functions/pivot.py
index b6b21f8..855f31d 100644
--- a/janitor/functions/pivot.py
+++ b/janitor/functions/pivot.py
@@ -1443,7 +1443,7 @@ def pivot_wider(
         3  6.1     2  18  29
 
         Pivot and flatten columns:
-        >>> df.pivot_wider(
+        >>> df.pivot_wider( # doctest: +SKIP
         ...     index = "dep",
         ...     names_from = "step",
         ... )
@@ -1452,7 +1452,7 @@ def pivot_wider(
         1  6.1   22   18   19   29
 
         Modify columns with `names_sep`:
-        >>> df.pivot_wider(
+        >>> df.pivot_wider( # doctest: +SKIP
         ...     index = "dep",
         ...     names_from = "step",
         ...     names_sep = "",
@@ -1462,7 +1462,7 @@ def pivot_wider(
         1  6.1  22  18  19  29
 
         Modify columns with `names_glue`:
-        >>> df.pivot_wider(
+        >>> df.pivot_wider( # doctest: +SKIP
         ...     index = "dep",
         ...     names_from = "step",
         ...     names_glue = "{_value}_step{step}",
@@ -1489,10 +1489,10 @@ def pivot_wider(
         0  Thu      3
         0  Fri      1
         0  Mon      5
-        >>> daily.pivot_wider(names_from='day', values_from='value')
+        >>> daily.pivot_wider(names_from='day', values_from='value') # doctest: +SKIP
            Tue  Thu  Fri  Mon
         0    2    3    1    5
-        >>> (daily
+        >>> (daily # doctest: +SKIP
         ... .pivot_wider(
         ...     names_from='day',
         ...     values_from='value',
@@ -1510,13 +1510,13 @@ def pivot_wider(
         0  Thu      3      B
         0  Fri      1      B
         0  Mon      5      A
-        >>> daily.pivot_wider(index='day',names_from='letter',values_from='value')
+        >>> daily.pivot_wider(index='day',names_from='letter',values_from='value') # doctest: +SKIP
            day    A    B
         0  Tue  2.0  NaN
         1  Thu  NaN  3.0
         2  Fri  NaN  1.0
         3  Mon  5.0  NaN
-        >>> (daily
+        >>> (daily # doctest: +SKIP
         ... .pivot_wider(
         ...     index='day',
         ...     names_from='letter',


Test patch:
diff --git a/tests/functions/test_coalesce.py b/tests/functions/test_coalesce.py
index f17a5f6..e2b0d26 100644
--- a/tests/functions/test_coalesce.py
+++ b/tests/functions/test_coalesce.py
@@ -63,3 +63,22 @@ def test_coalesce_without_delete():
     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)
+
+
+def test_coalesce_duplicate_columns():
+    """
+    Test output on duplicate columns.
+    """
+    df = pd.DataFrame(
+        np.array([[1.0, 2.0, 2.0], [np.nan, 3.0, np.nan], [3.0, 1.0, 9.0]]),
+        columns=["a", "a", "c"],
+    )
+
+    expected = pd.DataFrame(
+        np.array([[1.0, 2.0, 2.0], [3, 3.0, np.nan], [3.0, 1.0, 9.0]]),
+        columns=["a", "a", "c"],
+    )
+
+    actual = df.coalesce("a")
+
+    assert_frame_equal(expected, actual)