Repository: feature-engine/feature_engine
Pull request #596 (2023-01-16 15:21:13)

fixed bug in drop_features
Hi,

I found a bug in the DropFeatures transformer, module drop_features.py. The check (number of columns) in the .fit() method may fail unintentionally if features_to_drop is a string.

```
from feature_engine.selection import DropFeatures
import pandas as pd

df = pd.DataFrame({'aa':[1,2], 'bb': [3,4]})
transformer = DropFeatures(features_to_drop="aa")
X = transformer.fit_transform(df)

```

To fix this, I´ve applied the conversion of features_to_drop using the _find_all_variables method.


solegalli: Hey @luismavs 

Thanks! There are still some codestyle errors.

Do you have installed a tool called [black](https://pypi.org/project/black/)?

If you do, you just need to execute `black feature-engine tests` and it will resolve it automatically (almost always).

To reproduce the errors locally, you need to install [flake](https://pypi.org/project/flake8/) and then execute `flake8 feature-engine tests`

Check the docs [here](https://feature-engine.readthedocs.io/en/latest/contribute/contribute_code.html#test-code-style)

These are the errors if you want to resolve manually:

```
feature_engine/selection/drop_features.py:86:56: W291 trailing whitespace
feature_engine/selection/drop_features.py:87:53: E128 continuation line under-indented for visual indent
feature_engine/selection/drop_features.py:112:1: W391 blank line at end of file
tests/test_selection/test_drop_features.py:38:1: E303 too many blank lines (3)
tests/test_selection/test_drop_features.py:98:1: W391 blank line at end of file
stylechecks: exit 1 (3.46 seconds) /home/circleci/project> flake8 feature_engine tests pid=454
  stylechecks: FAIL code 1 (6.24=setup[2.78]+cmd[3.46] seconds)
  evaluation failed :( (6.33 seconds)


Exited with code exit status 1

CircleCI received exit code 1

```
codecov[bot]: # [Codecov](https://codecov.io/gh/feature-engine/feature_engine/pull/596?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=feature-engine) Report
> Merging [#596](https://codecov.io/gh/feature-engine/feature_engine/pull/596?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=feature-engine) (40ed8b5) into [main](https://codecov.io/gh/feature-engine/feature_engine/commit/a0106c968b14922d5123aaf51625fa66123dfad8?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=feature-engine) (a0106c9) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.

```diff
@@           Coverage Diff           @@
##             main     #596   +/-   ##
=======================================
  Coverage   97.79%   97.79%           
=======================================
  Files          90       90           
  Lines        3496     3497    +1     
  Branches      684      684           
=======================================
+ Hits         3419     3420    +1     
  Misses         29       29           
  Partials       48       48           
```


| [Impacted Files](https://codecov.io/gh/feature-engine/feature_engine/pull/596?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=feature-engine) | Coverage Δ | |
|---|---|---|
| [feature\_engine/encoding/decision\_tree.py](https://codecov.io/gh/feature-engine/feature_engine/pull/596?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=feature-engine#diff-ZmVhdHVyZV9lbmdpbmUvZW5jb2RpbmcvZGVjaXNpb25fdHJlZS5weQ==) | `100.00% <ø> (ø)` | |
| [feature\_engine/selection/drop\_features.py](https://codecov.io/gh/feature-engine/feature_engine/pull/596?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=feature-engine#diff-ZmVhdHVyZV9lbmdpbmUvc2VsZWN0aW9uL2Ryb3BfZmVhdHVyZXMucHk=) | `100.00% <100.00%> (ø)` | |

:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=feature-engine)

luismavs: Hey, code has now been properly formatted  with black, looks like good to go.

Patch:
diff --git a/feature_engine/selection/drop_features.py b/feature_engine/selection/drop_features.py
index b4ecd80..708bedd 100644
--- a/feature_engine/selection/drop_features.py
+++ b/feature_engine/selection/drop_features.py
@@ -2,6 +2,9 @@ from typing import List, Union
 
 import pandas as pd
 
+from feature_engine._variable_handling.variable_type_selection import (
+    _find_all_variables,
+)
 from feature_engine.dataframe_checks import check_X
 from feature_engine.selection.base_selector import BaseSelector
 from feature_engine.tags import _return_tags
@@ -54,7 +57,6 @@ class DropFeatures(BaseSelector):
     """
 
     def __init__(self, features_to_drop: List[Union[str, int]]):
-
         if not isinstance(features_to_drop, (str, list)) or len(features_to_drop) == 0:
             raise ValueError(
                 f"features_to_drop should be a list with the name of the variables "
@@ -81,10 +83,10 @@ class DropFeatures(BaseSelector):
         # present in the df.
         X[self.features_to_drop]
 
-        self.features_to_drop_ = self.features_to_drop
+        self.features_to_drop_ = _find_all_variables(X, variables=self.features_to_drop)
 
         # check user is not removing all columns in the dataframe
-        if len(self.features_to_drop) == len(X.columns):
+        if len(self.features_to_drop_) == len(X.columns):
             raise ValueError(
                 "The resulting dataframe will have no columns after dropping all "
                 "existing variables"


Test patch:
diff --git a/tests/test_selection/test_drop_features.py b/tests/test_selection/test_drop_features.py
index 49cb8db..70acfd9 100644
--- a/tests/test_selection/test_drop_features.py
+++ b/tests/test_selection/test_drop_features.py
@@ -27,6 +27,32 @@ def test_drop_1_variable(df_vartypes):
     pd.testing.assert_frame_equal(X, df)
 
 
+def test_drop_1_variables_str_input(df_vartypes):
+    """check that the internal variable features_to_drop has been
+     correctly cast from :str: to list of :str: internally"""
+
+    transformer = DropFeatures(features_to_drop="Marks")
+    X = transformer.fit_transform(df_vartypes)
+
+    # expected result
+    df = pd.DataFrame(
+        {
+            "Name": ["tom", "nick", "krish", "jack"],
+            "City": ["London", "Manchester", "Liverpool", "Bristol"],
+            "Age": [20, 21, 19, 18],
+            "dob": pd.date_range("2020-02-24", periods=4, freq="T"),
+        }
+    )
+
+    # init params
+    assert transformer.features_to_drop == "Marks"
+
+    # transform params
+    assert X.shape == (4, 4)
+    assert type(X) == pd.DataFrame
+    pd.testing.assert_frame_equal(X, df)
+
+
 def test_drop_2_variables(df_vartypes):
     transformer = DropFeatures(features_to_drop=["City", "dob"])
     X = transformer.fit_transform(df_vartypes)