Add IPW diagnostics tests for fit parameter reporting

[neuralsorcerer]

No description provided.

[Copilot]

Pull request overview

This PR strengthens the balance package’s IPW diagnostics test coverage by refactoring repeated IPW test setup into a shared helper and adding new assertions around how fitted model parameters are surfaced (or intentionally filtered/coerced) in Sample.diagnostics().

Changes:

• Refactor repeated IPW sample/target construction in diagnostics tests into _make_adjusted_ipw_sample().
• Add new diagnostics tests for solver/penalty reporting, filtering of non-string values, and NaN coercion for non-scalar tol/l1_ratio.
• Remove obsolete TODO comments in Sample.diagnostics() and document the expanded coverage in CHANGELOG.md.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
tests/test_sample.py	Adds/refactors IPW diagnostics tests and introduces additional edge-case assertions for model fit parameter reporting.
balance/sample_class.py	Removes TODO comments now covered by tests (no functional change).
CHANGELOG.md	Notes expanded IPW diagnostics fit-parameter reporting coverage.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

tests/test_sample.py:2530

• test_diagnostics_includes_n_iter_intercept claims to verify both n_iter_ and intercept_ are included, but the assertion only checks that any diagnostics key contains one of the substrings. This can pass even if only one of the two fields is reported. Consider asserting the presence (and ideally non-null values) of both expected rows, e.g. ("ipw_model_glance", "n_iter_") and ("ipw_model_glance", "intercept_") in the diagnostics index.

        diagnostics_dict = diagnostics.set_index(["metric", "var"])["val"].to_dict()
        self.assertTrue(
            any(
                "ipw_model_glance" in str(k)
                or "n_iter_" in str(k)

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

tests/test_sample.py:2531

• test_diagnostics_includes_n_iter_intercept assertion is too weak: it will pass as long as any key contains "ipw_model_glance" (which is expected even when n_iter_/intercept_ are missing). This means the test may not actually verify that n_iter_ and intercept_ rows are present. Consider asserting the specific (metric, var) pairs exist (e.g., ("ipw_model_glance", "n_iter_") and ("ipw_model_glance", "intercept_")) and optionally validate their values are scalar.

        self.assertTrue(
            any(
                "ipw_model_glance" in str(k)
                or "n_iter_" in str(k)
                or "intercept_" in str(k)

[meta-codesync]

@talgalili has imported this pull request. If you are a Meta employee, you can view this in D95205869.

[meta-codesync]

@talgalili merged this pull request in 6562849.