Repository: cytomining/pycytominer

Issue:
Bug: `samples` parameter in `feature_select` function reduces the rows in output
### Example code with output

Example code:
```python
    # Only for Plate 6, we want to normalize to iNFixion institution and Null and WT cells 
    # to keep consistent with how the other plates are normalized (same cell line)
    if plate == "Plate_6":
        samples = "Metadata_Institution == 'iNFixion' and (Metadata_genotype == 'Null' or Metadata_genotype == 'WT')"

    print(f"Performing normalization for {plate} using samples parameter: {samples}")

    # Step 2: Normalization
    normalized_df = normalize(
        profiles=output_annotated_file,
        method="standardize",
        samples=samples,
    )
    print("Normalized dataframe shape", normalized_df.shape)

    output(
        df=normalized_df,
        output_filename=output_normalized_file,
        output_type="parquet",
    )

    # Step 3: Feature selection
    feature_select_df = feature_select(
        output_normalized_file,
        operation=feature_select_ops,
        na_cutoff=0,
        samples=samples,
    )

    print("Feature selected dataframe shape", feature_select_df.shape)

    output(
        df=feature_select_df,
        output_filename=output_feature_select_file,
        output_type="parquet",
    )
```
Output:
```
Normalized dataframe shape (7383, 2319)
Feature selected dataframe shape (1674, 1148)
```


### Issue description

When using the `samples` parameter in the `feature_select` function, with the help of @gwaybio, we noticed that the number of rows reduced significantly. This did not happen if the `samples` parameter is removed, meaning it is defaulted to "all". 

Through talking with @axiomcura, it was determined that the issue was related to this part of the code:

```python
if samples != "all":
    population_df.query(samples, inplace=True)
```

When using query in this way, it removes all the other samples/rows in the dataframe, which means the output will be incorrect. 

NOTE: This parameter is also seen in the `normalize` function but it does not have this issue.

### Expected behavior

The expected behavior would be that the `feature_select` function will return a data frame with the same number of rows as the input when the `samples` parameter is used but will only determine which columns to keep based on only the samples specified.

### Additional information

- Pycytominer v1.1.0
- Linux operating system; Pop OS!
- Running in a jupyter notebook

Discussion:
codecov-commenter: ## [Codecov](https://app.codecov.io/gh/cytomining/pycytominer/pull/495?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cytomining) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 94.41%. Comparing base [(`5743c62`)](https://app.codecov.io/gh/cytomining/pycytominer/commit/5743c62d790376950bb77fb797c8a556c76e60c6?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cytomining) to head [(`f205458`)](https://app.codecov.io/gh/cytomining/pycytominer/commit/f205458d347723ed13bbbc27f845d7ea87990269?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cytomining).

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


```diff
@@            Coverage Diff             @@
##             main     #495      +/-   ##
==========================================
+ Coverage   94.39%   94.41%   +0.02%     
==========================================
  Files          57       57              
  Lines        3159     3171      +12     
==========================================
+ Hits         2982     2994      +12     
  Misses        177      177              
```

| [Flag](https://app.codecov.io/gh/cytomining/pycytominer/pull/495/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cytomining) | Coverage Δ | |
|---|---|---|
| [unittests](https://app.codecov.io/gh/cytomining/pycytominer/pull/495/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cytomining) | `94.41% <100.00%> (+0.02%)` | :arrow_up: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cytomining#carryforward-flags-in-the-pull-request-comment) to find out more.

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/cytomining/pycytominer/pull/495?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cytomining).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cytomining).

gwaybio: This looks functionally good to me @axiomcura - kudos and thanks! @d33bs could you give a quick, but close look to see if there are any technical things for Erik to consider prior to merging? One thing that I could potentially see is to create a new test (rather than append to an existing one), which would be more explicitly named to test the sample parameter (e.g., `test_feature_select_samples_paramater()`), but I'm not sure how hard or important this would be.

Patch:
diff --git a/.gitignore b/.gitignore
index 0191f56..e0a2b14 100644
--- a/.gitignore
+++ b/.gitignore
@@ -168,3 +168,6 @@ cython_debug/
 
 # Miscellaneous
 .DS_Store
+
+# removes files that are meant for prototyping
+_*
diff --git a/pycytominer/cyto_utils/features.py b/pycytominer/cyto_utils/features.py
index e3e95c5..57eaad7 100644
--- a/pycytominer/cyto_utils/features.py
+++ b/pycytominer/cyto_utils/features.py
@@ -173,19 +173,29 @@ def drop_outlier_features(
         Features greater than the threshold.
     """
 
-    # Subset dataframe
+    # Subset the DataFrame if specific samples are specified
+    # If "all", use the entire DataFrame without subsetting
     if samples != "all":
-        population_df.query(samples, inplace=True)
+        # Using pandas query to filter rows based on the conditions provided in the
+        # samples parameter
+        population_df = population_df.query(expr=samples)
 
+    # Infer  CellProfiler features if 'features' is set to 'infer'
     if features == "infer":
+        # Infer CellProfiler features
         features = infer_cp_features(population_df)
+        # Subset the DataFrame to only include inferred CellProfiler features
         population_df = population_df.loc[:, features]
     else:
+        # Subset the DataFrame to only include the features of interest
+        # this would be more tailored to non-CellProfiler features
         population_df = population_df.loc[:, features]
 
+    # Get the max and min values for each feature
     max_feature_values = population_df.max().abs()
     min_feature_values = population_df.min().abs()
 
+    # Identify features with max or min values greater than the outlier cutoff
     outlier_features = max_feature_values[
         (max_feature_values > outlier_cutoff) | (min_feature_values > outlier_cutoff)
     ].index.tolist()
diff --git a/pycytominer/operations/correlation_threshold.py b/pycytominer/operations/correlation_threshold.py
index a888a01..bc247ee 100644
--- a/pycytominer/operations/correlation_threshold.py
+++ b/pycytominer/operations/correlation_threshold.py
@@ -40,19 +40,26 @@ def correlation_threshold(
          List of features to exclude from the population_df.
     """
 
-    # Check that the input method is supported
+    # Checking if the provided correlation method is supported
     method = check_correlation_method(method)
 
+    # Checking if the threshold is between 0 and 1
     if not 0 <= threshold <= 1:
         raise ValueError("threshold variable must be between (0 and 1)")
 
     # Subset dataframe and calculate correlation matrix across subset features
+    # If samples is not 'all', then subset the dataframe
     if samples != "all":
-        population_df.query(samples, inplace=True)
+        # Using pandas query to filter rows based on the conditions provided in the
+        # samples parameter
+        population_df = population_df.query(expr=samples)
 
+    # Infer CellProfiler features if 'features' is set to 'infer'
     if features == "infer":
+        # Infer CellProfiler features
         features = infer_cp_features(population_df)
 
+    # Subset the DataFrame to only include the features of interest
     population_df = population_df.loc[:, features]
 
     # Get correlation matrix and lower triangle of pairwise correlations in long format
diff --git a/pycytominer/operations/get_na_columns.py b/pycytominer/operations/get_na_columns.py
index f288f2c..6c979ee 100644
--- a/pycytominer/operations/get_na_columns.py
+++ b/pycytominer/operations/get_na_columns.py
@@ -32,16 +32,29 @@ def get_na_columns(population_df, features="infer", samples="all", cutoff=0.05):
          List of features to exclude from the population_df.
     """
 
+    # Checking if the cutoff is between 0 and 1
+    if not 0 <= cutoff <= 1:
+        raise ValueError("cutoff variable must be between (0 and 1)")
+
+    # Subset the DataFrame if specific samples are specified
+    # If "all", use the entire DataFrame without subsetting
     if samples != "all":
-        population_df.query(samples, inplace=True)
+        # Using pandas query to filter rows based on the conditions provided in the
+        # samples parameter
+        population_df = population_df.query(expr=samples)
 
+    # Infer  CellProfiler features if 'features' is set to 'infer'
     if features == "infer":
+        # Infer CellProfiler features
         features = infer_cp_features(population_df)
 
+    # Subset the DataFrame to only include the features of interest
     population_df = population_df.loc[:, features]
 
+    # Get the proportion of NA values for each feature
     num_rows = population_df.shape[0]
     na_prop_df = population_df.isna().sum() / num_rows
 
+    # Get the features that have more NA values than the cutoff
     na_prop_df = na_prop_df[na_prop_df > cutoff]
     return list(set(na_prop_df.index.tolist()))
diff --git a/pycytominer/operations/noise_removal.py b/pycytominer/operations/noise_removal.py
index e3e4192..9f0172a 100644
--- a/pycytominer/operations/noise_removal.py
+++ b/pycytominer/operations/noise_removal.py
@@ -41,30 +41,43 @@ def noise_removal(
         A list of features to be removed, due to having too high standard deviation within replicate groups.
 
     """
-    # Subset dataframe
+
+    # Subset the DataFrame if specific samples are specified
+    # If "all", use the entire DataFrame without subsetting
     if samples != "all":
-        population_df.query(samples, inplace=True)
+        # Using pandas query to filter rows based on the conditions provided in the
+        # samples parameter
+        population_df = population_df.query(expr=samples)
 
+    # Infer  CellProfiler features if 'features' is set to 'infer'
     if features == "infer":
+        # Infer CellProfiler features
         features = infer_cp_features(population_df)
+        # Subset the DataFrame to only include inferred CellProfiler features
 
-    # If a metadata column name is specified, use that as the perturb groups
+    # if a Metadata columns name is specified, use that as the perturb groups
     if isinstance(noise_removal_perturb_groups, str):
+        # Check if the column exists
         if noise_removal_perturb_groups not in population_df.columns:
             raise ValueError(
                 'f"{perturb} not found. Are you sure it is a ' "metadata column?"
             )
+        # Assign the group info to the specified column
         group_info = population_df[noise_removal_perturb_groups]
 
     # Otherwise, the user specifies a list of perturbs
     elif isinstance(noise_removal_perturb_groups, list):
+        # Check if the length of the noise_removal_perturb_groups is the same as the
+        # number of rows in the df
         if not len(noise_removal_perturb_groups) == len(population_df):
             raise ValueError(
                 f"The length of input list: {len(noise_removal_perturb_groups)} is not equivalent to your "
                 f"data: {population_df.shape[0]}"
             )
+        # Assign the group info to the the noise_removal_perturb_groups
         group_info = noise_removal_perturb_groups
     else:
+        # Raise an error if the input is not a list or a string
         raise TypeError(
             "noise_removal_perturb_groups must be a list corresponding to row perturbations or a str \
                         specifying the name of the metadata column."
@@ -74,10 +87,16 @@ def noise_removal(
     population_df = population_df.loc[:, features]
     population_df = population_df.assign(group_id=group_info)
 
-    # Get the standard deviations of features within each group
+    # Get the standard deviations of features within each group then calculate the mean
+    # of these standard deviations.
+    # This tells us how much the standard deviation of each feature varies within each
+    # perturbation group.
     stdev_means_df = population_df.groupby("group_id").std(ddof=0).mean()
 
-    # Identify noisy features with a greater mean stdev within perturbation group than the threshold
+    # With the stdev_means_df, we can identify features that have a mean stdev greater than
+    # the cutoff
+    # These features are considered to have too much variation within replicate groups
+    # and are removed. This returns a list of features to remove.
     to_remove = stdev_means_df[
         stdev_means_df > noise_removal_stdev_cutoff
     ].index.tolist()
diff --git a/pycytominer/operations/variance_threshold.py b/pycytominer/operations/variance_threshold.py
index 72da751..3d0f52e 100644
--- a/pycytominer/operations/variance_threshold.py
+++ b/pycytominer/operations/variance_threshold.py
@@ -44,37 +44,57 @@ def variance_threshold(
 
     """
 
+    # check if freq_cut and unique_cut are between 0 and 1
     if not 0 <= freq_cut <= 1:
         raise ValueError("freq_cut variable must be between (0 and 1)")
     if not 0 <= unique_cut <= 1:
         raise ValueError("unique_cut variable must be between (0 and 1)")
 
-    # Subset dataframe
+    # Subset the DataFrame if specific samples are specified
+    # If "all", use the entire DataFrame without subsetting
     if samples != "all":
-        population_df.query(samples, inplace=True)
+        # Using pandas query to filter rows based on the conditions provided in the
+        # samples parameter
+        population_df = population_df.query(expr=samples)
 
+    # Infer CellProfiler features if 'features' is set to 'infer'
     if features == "infer":
+        # Infer CellProfiler features
         features = infer_cp_features(population_df)
 
+    # Subset the DataFrame to only include the features of interest
     population_df = population_df.loc[:, features]
 
-    # Exclude features with extreme (defined by freq_cut ratio) common values
+    # Exclude features based on frequency
+    # Frequency is the ratio of the second most common value to the most common value.
+    # Features with a frequency below the `freq_cut` threshold are flagged for exclusion.
     excluded_features_freq = population_df.apply(
         lambda x: calculate_frequency(x, freq_cut), axis="rows"
     )
 
+    # Remove features with NA values
     excluded_features_freq = excluded_features_freq[
         excluded_features_freq.isna()
     ].index.tolist()
 
-    # Exclude features with too many (defined by unique_ratio) values in common
+    # Get the number of samples
     n = population_df.shape[0]
+
+    # Get the number of unique features
     num_unique_features = population_df.nunique()
 
+    # Exclude features with too many (defined by unique_ratio) values in common, where
+    # unique_ratio is defined as the number of unique features divided by the total
+    # number of samples
     unique_ratio = num_unique_features / n
     unique_ratio = unique_ratio < unique_cut
+
+    # Get the feature names that have a unique ratio less than the unique_cut
+    # This represents features that have too few unique values compared to the number
+    # of samples.
     excluded_features_unique = unique_ratio[unique_ratio].index.tolist()
 
+    # Combine the two lists of features to exclude
     excluded_features = list(set(excluded_features_freq + excluded_features_unique))
     return excluded_features
 


Test patch:
diff --git a/tests/test_feature_select.py b/tests/test_feature_select.py
index 8cc6a20..97fe265 100644
--- a/tests/test_feature_select.py
+++ b/tests/test_feature_select.py
@@ -494,3 +494,65 @@ def test_output_type():
 
     # check to make sure both dataframes are the same regardless of the output_type
     pd.testing.assert_frame_equal(csv_df, parquet_df)
+
+
+def test_samples_parameter_in_feature_select():
+    # Create a list of 100 elements with 10 replicates of 10 perturbations
+    data_unique_test_df_groups = ["a", "b", "c", "d", "e", "f", "g", "h", "i", "j"]
+    data_unique_test_df_groups = [
+        elem for elem in data_unique_test_df_groups for _ in range(10)
+    ]
+
+    # Make a copy of the test DataFrame
+    data_unique_test_df3 = data_unique_test_df.copy()
+    data_unique_test_df3_features = data_unique_test_df3.columns.tolist()
+
+    # Add a metadata column for creating a sample query
+    data_unique_test_df3["Metadata_sample"] = ["A", "B"] * 50
+
+    # Define the sample query
+    sample_query = "Metadata_sample == 'A'"
+
+    # Add a perturb group metadata column for noise removal operation
+    data_unique_test_df3["perturb_group"] = data_unique_test_df_groups
+
+    # establishing all operations that use "samples" parameter
+    # note that blocklist does not use samples parameter
+    all_operations = [
+        "noise_removal",
+        "drop_na_columns",
+        "variance_threshold",
+        "correlation_threshold",
+        "drop_outliers",
+    ]
+    for operation_idx, operation in enumerate(all_operations):
+        # testing single operation
+        results6a = feature_select(
+            profiles=data_unique_test_df3,
+            features=data_unique_test_df3_features,
+            operation=operation,
+            samples=sample_query,
+            noise_removal_perturb_groups="perturb_group",
+            noise_removal_stdev_cutoff=500,
+        )
+
+        # checking if no rows were not removed
+        assert (
+            results6a.shape[0] == data_unique_test_df3.shape[0]
+        ), f"Row counts do not match: {results6a[0]} != {data_unique_test_df3.shape[0]} in operation: {operation}"
+
+        # testing multiple operations (continually appends operations)
+        concat_operations = all_operations[: operation_idx + 1]
+        results6b = feature_select(
+            profiles=data_unique_test_df3,
+            features=data_unique_test_df3_features,
+            operation=concat_operations,
+            samples=sample_query,
+            noise_removal_perturb_groups="perturb_group",
+            noise_removal_stdev_cutoff=500,
+        )
+
+        # checking if no rows were not removed
+        assert (
+            results6b.shape[0] == data_unique_test_df3.shape[0]
+        ), f"Row counts do not match: {results6a[0]} != {data_unique_test_df3.shape[0]} in operation: {concat_operations}"
diff --git a/tests/test_operations/test_get_na_columns.py b/tests/test_operations/test_get_na_columns.py
index 48b5ab7..acaa9a5 100644
--- a/tests/test_operations/test_get_na_columns.py
+++ b/tests/test_operations/test_get_na_columns.py
@@ -73,3 +73,18 @@ def test_get_na_columns_featureinfer():
         )
 
         assert "No features found." in str(nocp.value)
+
+
+def test_get_na_columns_cutoff_parameter():
+    # Define invalid cutoff values
+    invalid_cutoffs = [10, -0.1]
+
+    # Test cutoff parameter
+    for cutoff in invalid_cutoffs:
+        # Test if ValueError is raised with invalid cutoff values
+        with pytest.raises(
+            ValueError, match="cutoff variable must be between \\(0 and 1\\)"
+        ):
+            get_na_columns(
+                population_df=data_df, samples="all", features="infer", cutoff=cutoff
+            )