Address PR #346 CI review round 10: P1 sklearn-compatible get/set_params

**P1 (Code Quality): sklearn parameter contract**

The class docstring advertises `sklearn.base.clone(est)` compatibility,
but the actual methods did not match sklearn's
`BaseEstimator.get_params(deep=True)` / `set_params(**params)` surface:

- `get_params()` did not accept a `deep` keyword. sklearn's `clone`
  calls `get_params(deep=False)`, so any integration would have
  failed with a TypeError on the missing kwarg.
- `set_params()` validated keys with `hasattr(self, key)`. That would
  silently accept non-constructor attribute names like `fit`, and a
  typo or malicious kwargs dict could overwrite estimator methods.

Fix:
- `get_params(self, deep: bool = True)` matches sklearn's signature.
  `deep` is accepted for compat; this estimator has no nested
  sub-estimators, so `deep=True` and `deep=False` return the same
  dict. `del deep` documents the no-op explicitly and silences
  unused-arg linters.
- `set_params(**params)` now restricts to keys from `get_params()`.
  Non-constructor attribute names (including method names like `fit`
  and dunder/private attrs) raise `ValueError`.

**Tests (+4 regression):**
- test_set_params_rejects_method_names: `set_params(fit=...)` raises
  and `est.fit` stays callable.
- test_set_params_rejects_private_attrs: `_internal=42` raises.
- test_get_params_accepts_deep_keyword: `deep=True`, `deep=False`, and
  no-arg all return the same dict.
- test_sklearn_clone_round_trip_if_available: `sklearn.base.clone`
  round-trips the estimator; gated on `pytest.importorskip("sklearn")`
  so it skips cleanly when sklearn is not in the test matrix.

Targeted regression: 155 HAD tests (+ 1 skipped) + 534 total (+1
skipped) across Phase 1 and adjacent surfaces, all green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: igerber

diff_diff/had.py

@@ -994,13 +994,23 @@ def _validate_constructor_args(self) -> None:
                     f"or None."
                 )
 
-    def get_params(self) -> Dict[str, Any]:
+    def get_params(self, deep: bool = True) -> Dict[str, Any]:
         """Return the raw constructor parameters (sklearn-compatible).
 
-        Preserves the user's original inputs - in particular, ``design``
-        returns ``"auto"`` when the user set it to ``"auto"`` (even after
-        fit), so ``sklearn.base.clone(est)`` round-trips exactly.
+        Matches the :meth:`sklearn.base.BaseEstimator.get_params`
+        signature. Preserves the user's original inputs - in particular,
+        ``design`` returns ``"auto"`` when the user set it to ``"auto"``
+        (even after fit), so ``sklearn.base.clone(est)`` round-trips
+        exactly.
+
+        Parameters
+        ----------
+        deep : bool, default=True
+            Accepted for sklearn-contract compatibility. This estimator
+            has no nested sub-estimator parameters, so ``deep=False``
+            and ``deep=True`` return the same dict.
         """
+        del deep  # accepted for compat; this estimator has no nested params
         return {
             "design": self.design,
             "d_lower": self.d_lower,
@@ -1012,12 +1022,18 @@ def get_params(self) -> Dict[str, Any]:
         }
 
     def set_params(self, **params: Any) -> "HeterogeneousAdoptionDiD":
-        """Set estimator parameters and return self (sklearn-compatible)."""
+        """Set estimator parameters and return self (sklearn-compatible).
+
+        Only keys returned by :meth:`get_params` are accepted. Passing
+        any other attribute name (including method names like ``fit``)
+        raises ``ValueError`` so the estimator cannot be silently
+        corrupted by a mistyped or attacker-supplied key.
+        """
+        valid_keys = set(self.get_params().keys())
         for key, value in params.items():
-            if not hasattr(self, key):
+            if key not in valid_keys:
                 raise ValueError(
-                    f"Invalid parameter: {key}. Valid parameters: "
-                    f"{list(self.get_params().keys())}."
+                    f"Invalid parameter: {key!r}. Valid parameters: " f"{sorted(valid_keys)}."
                 )
             setattr(self, key, value)
         self._validate_constructor_args()

tests/test_had.py

@@ -849,6 +849,46 @@ def test_set_params_invalid_key_raises(self):
         with pytest.raises(ValueError, match="Invalid parameter"):
             est.set_params(not_a_param=True)
 
+    def test_set_params_rejects_method_names(self):
+        """Review P1 round 10: set_params must restrict to constructor keys,
+        not any hasattr-able name. Method names like 'fit' must raise,
+        else they would silently overwrite the method.
+        """
+        est = HeterogeneousAdoptionDiD()
+        with pytest.raises(ValueError, match="Invalid parameter"):
+            est.set_params(fit="not_a_method")
+        # sanity: fit is still callable on the class
+        assert callable(est.fit)
+
+    def test_set_params_rejects_private_attrs(self):
+        """Internal-looking attribute names must also raise."""
+        est = HeterogeneousAdoptionDiD()
+        with pytest.raises(ValueError, match="Invalid parameter"):
+            est.set_params(_internal=42)
+
+    def test_get_params_accepts_deep_keyword(self):
+        """Review P1 round 10: get_params must match sklearn's signature.
+
+        sklearn.base.BaseEstimator.get_params(deep=True). This estimator
+        has no nested sub-estimators, so deep=True and deep=False return
+        the same dict, but the keyword must be accepted.
+        """
+        est = HeterogeneousAdoptionDiD(design="continuous_at_zero", alpha=0.1)
+        params_default = est.get_params()
+        params_deep_true = est.get_params(deep=True)
+        params_deep_false = est.get_params(deep=False)
+        assert params_default == params_deep_true == params_deep_false
+
+    def test_sklearn_clone_round_trip_if_available(self):
+        """If sklearn is installed, sklearn.base.clone round-trips the estimator."""
+        sklearn_base = pytest.importorskip("sklearn.base")
+        est = HeterogeneousAdoptionDiD(design="auto", alpha=0.1, kernel="triangular")
+        cloned = sklearn_base.clone(est)
+        assert cloned.get_params() == est.get_params()
+        assert cloned is not est
+        # clone produces a fresh instance of the same class.
+        assert type(cloned) is type(est)
+
     def test_set_params_invalid_design_raises(self):
         est = HeterogeneousAdoptionDiD()
         with pytest.raises(ValueError, match="design"):