Repository: fairlearn/fairlearn

Issue:
MetricFrame and multiple/named sensitive features
I was writing an example notebook, and I encountered something which I found strange. Basically, this works:

``` python
multi_metric = MetricFrame(
    {
        "average precision": skm.average_precision_score,
        "roc-auc": skm.roc_auc_score,
        "count": lambda x, y: len(x),
    },
    y_test,
    y_pred,
    sensitive_features=gender_test,
)
```

But this doesn't:

``` python
multi_metric = MetricFrame(
    {
        "average precision": skm.average_precision_score,
        "roc-auc": skm.roc_auc_score,
        "count": lambda x, y: len(x),
    },
    y_test,
    y_pred,
    sensitive_features={'gender': gender_test},
)
```
with error: 

    ValueError: If using all scalar values, you must pass an index

To me this error message is not clear at all, and I have no pandas DataFrames to have any indices in the first place. I don't think we should be failing in this case.

cc @riedgar-ms @MiroDudik 

Here's the whole notebook for reproducibility

<details>

``` python
# %%
# First we need to install the requirements:
# pip install model-card-toolkit scikit-learn matplotlib fairlear pandas

# %%
# Here we have all the imports required to run the rest of the script
import numpy as np
import sklearn.metrics as skm
from numpy.random import RandomState
from sklearn.datasets import make_classification
from sklearn.model_selection import train_test_split
from sklearn.experimental import enable_hist_gradient_boosting  # noqa
from sklearn.ensemble import HistGradientBoostingClassifier
from sklearn.decomposition import PCA
from sklearn.pipeline import make_pipeline
from fairlearn.metrics import MetricFrame

# %%
rng = RandomState(seed=42)

X_women, y_women = make_classification(
    n_samples=500,
    n_features=20,
    n_informative=4,
    n_classes=2,
    class_sep=1,
    random_state=rng,
)

X_men, y_men = make_classification(
    n_samples=500,
    n_features=20,
    n_informative=4,
    n_classes=2,
    class_sep=2,
    random_state=rng,
)

X_unspecified, y_unspecified = make_classification(
    n_samples=500,
    n_features=20,
    n_informative=4,
    n_classes=2,
    class_sep=0.5,
    random_state=rng,
)

X = np.r_[X_women, X_men, X_unspecified]
y = np.r_[y_women, y_men, y_unspecified]
gender = np.r_[["Woman"] * 500, ["Man"] * 500, ["Unspecified"] * 500].reshape(
    -1, 1
)
X_train, X_test, y_train, y_test, gender_train, gender_test = train_test_split(
    X, y, gender, test_size=0.3, random_state=rng
)

# %%
# Now we train a classifier on the data
clf = make_pipeline(PCA(n_components=4), HistGradientBoostingClassifier())
clf.fit(X_train, y_train)

# %%
# We can finally check the performance of our model for different groups.
y_pred = clf.predict_proba(X_test)[:, 0]
multi_metric = MetricFrame(
    {
        "average precision": skm.average_precision_score,
        "roc-auc": skm.roc_auc_score,
        "count": lambda x, y: len(x),
    },
    y_test,
    y_pred,
    sensitive_features={'gender': gender_test},
)
```

</details>

Discussion:
romanlutz: LGTM! I'll leave it open for another day in case @adrinjalali has a comment since you added him, too.
riedgar-ms: > LGTM! I'll leave it open for another day in case @adrinjalali has a comment since you added him, too.

It's @adrinjalali 's bug, so he definitely needs to review.

Patch:
diff --git a/CHANGES.md b/CHANGES.md
index e5206ff..7d2bc95 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -1,5 +1,11 @@
 # Changes
 
+### v0.6.2
+
+* Bugfix for `_merge_columns()` when using multiple sensitive features with
+  long names. This previously caused groups to get merged if the concatenation
+  of their string representations was identical until the cutoff limit.
+
 ### v0.6.1
 
 * Bugfix for `_create_group_metric_set()`. Fixes the list of metrics computed for regression
diff --git a/fairlearn/_input_validation.py b/fairlearn/_input_validation.py
index 03d5fe9..962b348 100644
--- a/fairlearn/_input_validation.py
+++ b/fairlearn/_input_validation.py
@@ -60,6 +60,7 @@ def _validate_and_reformat_input(X, y=None, expect_y=True, enforce_binary_labels
         the reformatting process, so mitigation methods should ideally use the input X instead
         of the returned X for training estimators and leave potential reformatting of X to the
         estimator.
+
     """
     if y is not None:
         # calling check_X_y with a 2-dimensional y causes a warning, so ensure it is 1-dimensional
@@ -128,16 +129,14 @@ def _merge_columns(feature_columns: np.ndarray) -> np.ndarray:
     -------
     numpy.ndarray
         One-dimensional array of merged columns
+
     """
     if not isinstance(feature_columns, np.ndarray):
         raise ValueError("Received argument of type {} instead of expected numpy.ndarray"
                          .format(type(feature_columns).__name__))
-    return np.apply_along_axis(
-        lambda row: _MERGE_COLUMN_SEPARATOR.join(
-            [str(row[i])
-                .replace("\\", "\\\\")  # escape backslash and separator
-                .replace(_MERGE_COLUMN_SEPARATOR,
-                         "\\" + _MERGE_COLUMN_SEPARATOR)
-                for i in range(len(row))]),
-        axis=1,
-        arr=feature_columns)
+    return pd.DataFrame(feature_columns) \
+        .apply(lambda row: _MERGE_COLUMN_SEPARATOR.join([str(row[i])
+               .replace("\\", "\\\\")  # escape backslash and separator
+               .replace(_MERGE_COLUMN_SEPARATOR,
+                        "\\" + _MERGE_COLUMN_SEPARATOR)
+               for i in range(len(row))]), axis=1).values
diff --git a/fairlearn/metrics/_metric_frame.py b/fairlearn/metrics/_metric_frame.py
index 42a12b8..bf0188b 100644
--- a/fairlearn/metrics/_metric_frame.py
+++ b/fairlearn/metrics/_metric_frame.py
@@ -16,6 +16,8 @@ logger = logging.getLogger(__name__)
 
 _SUBGROUP_COUNT_WARNING_THRESHOLD = 20
 
+_SF_DICT_CONVERSION_FAILURE = "DataFrame.from_dict() failed on sensitive features. " \
+    "Please ensure each array is strictly 1-D."
 _BAD_FEATURE_LENGTH = "Received a feature of length {0} when length {1} was expected"
 _SUBGROUP_COUNT_WARNING = "Found {0} subgroups. Evaluation may be slow"
 _FEATURE_LIST_NONSCALAR = "Feature lists must be of scalar types"
@@ -519,7 +521,10 @@ class MetricFrame:
             else:
                 raise ValueError(_FEATURE_LIST_NONSCALAR)
         elif isinstance(features, dict):
-            df = pd.DataFrame.from_dict(features)
+            try:
+                df = pd.DataFrame.from_dict(features)
+            except ValueError as ve:
+                raise ValueError(_SF_DICT_CONVERSION_FAILURE) from ve
             for i in range(len(df.columns)):
                 col_name = df.columns[i]
                 if not isinstance(col_name, str):


Test patch:
diff --git a/test/unit/metrics/test_metricframe_process_features.py b/test/unit/metrics/test_metricframe_process_features.py
index e24e787..db09c09 100644
--- a/test/unit/metrics/test_metricframe_process_features.py
+++ b/test/unit/metrics/test_metricframe_process_features.py
@@ -90,6 +90,20 @@ class TestSingleFeature():
             _ = target._process_features("Unused", raw_feature, y_true)
         assert execInfo.value.args[0] == msg
 
+    def test_from_dict_failure(self):
+        r_f, y_true = self._get_raw_data()
+        raw_feature = {'Mine!': np.asarray(r_f).reshape(-1, 1)}
+        target = _get_raw_MetricFrame()
+        msg = "DataFrame.from_dict() failed on sensitive features. "\
+            "Please ensure each array is strictly 1-D."
+        with pytest.raises(ValueError) as ve:
+            _ = target._process_features("Unused", raw_feature, y_true)
+        assert msg == ve.value.args[0]
+        assert ve.value.__cause__ is not None
+        assert isinstance(ve.value.__cause__, ValueError)
+        # Ensure we got the gnomic pandas message
+        assert ve.value.__cause__.args[0] == 'If using all scalar values, you must pass an index'
+
 
 class TestTwoFeatures():
     def _get_raw_data(self):
diff --git a/test/unit/metrics/test_metricframe_smoke.py b/test/unit/metrics/test_metricframe_smoke.py
index 72626c2..5f9e8fc 100644
--- a/test/unit/metrics/test_metricframe_smoke.py
+++ b/test/unit/metrics/test_metricframe_smoke.py
@@ -229,7 +229,6 @@ def test_1m_1sf_1cf_metric_dict(transform_y_t, transform_y_p):
 
 def test_1m_1_sf_sample_weights():
     """Check that sample weights are passed correctly to a single metric."""
-
     def multi_sp(y_t, y_p, p1, p2):
         """Metric to check passing of sample parameters.
 
@@ -270,7 +269,6 @@ def test_1m_1_sf_sample_weights():
 
 def test_2m_1sf_sample_weights():
     """Check that sample weights are passed correctly to two metrics."""
-
     def sp_is_sum(y_t, y_p, some_param_name):
         """Metric accepting a single sample parameter.
 
diff --git a/test/unit/postprocessing/test_threshold_optimizer_multiple_sensitive_features.py b/test/unit/postprocessing/test_threshold_optimizer_multiple_sensitive_features.py
new file mode 100644
index 0000000..240f0d1
--- /dev/null
+++ b/test/unit/postprocessing/test_threshold_optimizer_multiple_sensitive_features.py
@@ -0,0 +1,125 @@
+# Copyright (c) Microsoft Corporation and Fairlearn contributors.
+# Licensed under the MIT License.
+
+import numpy as np
+import pandas as pd
+
+from sklearn.metrics import balanced_accuracy_score, accuracy_score
+from sklearn.linear_model import LinearRegression
+from fairlearn.postprocessing import ThresholdOptimizer
+
+from fairlearn.metrics import (
+    MetricFrame,
+    false_positive_rate,
+    false_negative_rate
+)
+
+fairness_metrics = {
+    "accuracy": accuracy_score,
+    "false_positive_rate": false_positive_rate,
+    "positive_count": lambda true, pred: np.sum(true),
+    "false_negative_rate": false_negative_rate,
+    "negative_count": lambda true, pred: np.sum(1-true),
+    "balanced_accuracy": balanced_accuracy_score
+}
+
+
+def test_threshold_optimizer_multiple_sensitive_features():
+    # Create sensitive features so that the third column is the first two combined.
+    # Also, the name a2 is long since that caused bug #728.
+    # The bug caused the merged names to get cut off, resulting in multiple groups
+    # getting merged internally. To avoid that this test case checks even internal
+    # representations.
+    X = pd.DataFrame([
+        [0, 4], [6, 2], [1, 3], [10, 5], [1, 7], [-2, 1], [3, 10], [14, 5],
+        [1, 3], [1, 5], [1, 7], [-5, 9], [3, 13], [7, 1], [-8, 4], [9, 1]])
+    y = pd.Series([0, 1, 1, 0, 0, 0, 1, 0, 0, 1, 0, 0, 1, 1, 1, 0])
+    a1 = "a"
+    a2 = "a very very very very very very very long group name"
+    a3 = "a group name with commas ,, in , it"
+    a4 = "a group name with backslashes \\ in \\\\ it"
+    A = pd.DataFrame([[a1, a3, a1 + a3], [a1, a3, a1 + a3],
+                      [a2, a3, a2 + a3], [a2, a3, a2 + a3], [a2, a3, a2 + a3], [a2, a3, a2 + a3],
+                      [a2, a4, a2 + a4], [a2, a4, a2 + a4], [a2, a4, a2 + a4], [a2, a4, a2 + a4],
+                      [a2, a4, a2 + a4], [a2, a4, a2 + a4], [a2, a4, a2 + a4], [a2, a4, a2 + a4],
+                      [a1, a4, a1 + a4], [a1, a4, a1 + a4]],
+                     columns=['SF1', 'SF2', 'SF1+2'])
+
+    estimator = LinearRegression()
+    estimator.fit(X, y)
+
+    postprocess_est_multi = ThresholdOptimizer(
+        estimator=estimator,
+        constraints="demographic_parity",
+        objective="accuracy_score",
+        prefit=True
+    )
+    postprocess_est_combined = ThresholdOptimizer(
+        estimator=estimator,
+        constraints="demographic_parity",
+        objective="accuracy_score",
+        prefit=True
+    )
+
+    postprocess_est_multi.fit(X, y, sensitive_features=A.loc[:, ['SF1', 'SF2']])
+    postprocess_est_combined.fit(X, y, sensitive_features=A.loc[:, 'SF1+2'])
+
+    X_test = pd.concat([
+        pd.DataFrame([[5, 4], [7, 2], [0, 3], [1, 2], [-2, 9], [1, 1], [0, 5], [-3, 3]]),
+        X])
+    A_test = pd.concat([
+        pd.DataFrame([[a1, a3, a1 + a3], [a1, a3, a1 + a3],
+                      [a2, a3, a2 + a3], [a2, a3, a2 + a3],
+                      [a2, a4, a2 + a4], [a2, a4, a2 + a4],
+                      [a1, a4, a1 + a4], [a1, a4, a1 + a4]],
+                     columns=['SF1', 'SF2', 'SF1+2']),
+        A])
+    y_test = pd.concat([pd.Series([0, 1, 0, 1, 0, 1, 0, 1]), y])
+
+    y_pred_multi = postprocess_est_multi.predict(
+        X_test, sensitive_features=A_test.loc[:, ['SF1', 'SF2']], random_state=1)
+    y_pred_combined = postprocess_est_combined.predict(
+        X_test, sensitive_features=A_test.loc[:, 'SF1+2'], random_state=1)
+
+    metricframe_multi = MetricFrame(
+        fairness_metrics,
+        y_test,
+        y_pred_multi,
+        sensitive_features=A_test.loc[:, ['SF1', 'SF2']]
+    )
+
+    metricframe_combined = MetricFrame(
+        fairness_metrics,
+        y_test,
+        y_pred_combined,
+        sensitive_features=A_test.loc[:, 'SF1+2']
+    )
+
+    # multi - names after escaping
+    a3_escaped = a3.replace(',', '\\,')
+    a4_escaped = a4.replace('\\', '\\\\')
+    a13 = f"{a1},{a3_escaped}"
+    a14 = f"{a1},{a4_escaped}"
+    a23 = f"{a2},{a3_escaped}"
+    a24 = f"{a2},{a4_escaped}"
+
+    assert (metricframe_combined.overall == metricframe_multi.overall).all()
+
+    assert (metricframe_combined.by_group.loc[a1+a3] ==
+            metricframe_multi.by_group.loc[(a1, a3)]).all()
+    assert (metricframe_combined.by_group.loc[a2+a3] ==
+            metricframe_multi.by_group.loc[(a2, a3)]).all()
+    assert (metricframe_combined.by_group.loc[a1+a4] ==
+            metricframe_multi.by_group.loc[(a1, a4)]).all()
+    assert (metricframe_combined.by_group.loc[a2+a4] ==
+            metricframe_multi.by_group.loc[(a2, a4)]).all()
+
+    # comparing string representations of interpolation dicts is sufficient
+    assert str(postprocess_est_combined.interpolated_thresholder_.interpolation_dict[a1+a3]) == \
+        str(postprocess_est_multi.interpolated_thresholder_.interpolation_dict[a13])
+    assert str(postprocess_est_combined.interpolated_thresholder_.interpolation_dict[a1+a4]) == \
+        str(postprocess_est_multi.interpolated_thresholder_.interpolation_dict[a14])
+    assert str(postprocess_est_combined.interpolated_thresholder_.interpolation_dict[a2+a3]) == \
+        str(postprocess_est_multi.interpolated_thresholder_.interpolation_dict[a23])
+    assert str(postprocess_est_combined.interpolated_thresholder_.interpolation_dict[a2+a4]) == \
+        str(postprocess_est_multi.interpolated_thresholder_.interpolation_dict[a24])