Skip to content

Increase balance package test coverage#281

Closed
talgalili wants to merge 1 commit intofacebookresearch:mainfrom
talgalili:export-D90946146
Closed

Increase balance package test coverage#281
talgalili wants to merge 1 commit intofacebookresearch:mainfrom
talgalili:export-D90946146

Conversation

@talgalili
Copy link
Contributor

Summary:
The coverage report showed 98% overall coverage with 7 files having gaps. These tests cover previously untested edge cases including:

  • CLI exception handling paths for weighting failures
  • Sample class design effect diagnostics and IPW model parameters
  • CBPS optimization convergence warnings and constraint violation exceptions
  • Plotting functions with missing values, default parameters, and various dist_types
  • Distance metrics with empty numeric columns

Differential Revision: D90946146

Copilot AI review requested due to automatic review settings January 18, 2026 20:53
@meta-codesync
Copy link

meta-codesync bot commented Jan 18, 2026

@talgalili has exported this pull request. If you are a Meta employee, you can view the originating Diff in D90946146.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR increases test coverage for the balance package by adding tests for previously untested edge cases across multiple modules. The changes target 7 files with coverage gaps, focusing on exception handling, diagnostics, convergence warnings, plotting functions with various parameters, and distance metric edge cases.

Changes:

  • Added tests for CLI exception handling paths when weighting fails, batch processing, and IPW model parameter passing
  • Added tests for Sample class design effect diagnostics and IPW model parameters in diagnostics output
  • Added tests for CBPS optimization convergence warnings and constraint violation exceptions using mocking
  • Added tests for plotting functions covering default parameter behavior, missing value handling, and various dist_types
  • Added tests for distance metric functions with empty numeric columns after filtering

Reviewed changes

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

Show a summary per file
File Description
tests/test_weighted_comparisons_plots.py Tests for seaborn_plot_dist default names/dist_types, missing value handling, plotly_plot_density without weights, plotly_plot_qq with plot_it=True, and unsupported dist_type error handling
tests/test_stats_and_plots.py Tests for empty numeric column handling in wasserstein_distance, cramer_von_mises, and kolmogorov_smirnov; tests for conf_int_of_weighted_mean with DataFrame and relative_frequency_table with Series inputs
tests/test_sample.py Tests for weight_trimming_percentile in str, design_effect_diagnostics edge cases, and IPW model parameters in diagnostics output
tests/test_cli.py Tests for CLI exception handling with succeed_on_weighting_failure flag, IPW method with model kwargs, batch column processing, and main() function
tests/test_cbps.py Tests for CBPS optimization convergence warnings and constraint violation exceptions using unittest.mock to control scipy.optimize return values

Comment on lines +1594 to +1599
result.__getitem__ = lambda self, key: {
"success": np.bool_(True),
"message": "Optimization terminated successfully",
"x": x0,
"fun": 0.01,
}[key]
Copy link

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lambda function for getitem captures a variable named 'self' which shadows the MagicMock instance. This works but is confusing. Consider using a different parameter name (e.g., 'obj' or '_') to avoid shadowing: result.__getitem__ = lambda obj, key: {...}[key]

Copilot uses AI. Check for mistakes.
Comment on lines +1614 to +1624
try:
balance_cbps.cbps(
sample_df,
sample_weights,
target_df,
target_weights,
transformations=None,
cbps_method="over",
)
except Exception:
pass
Copy link

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test catches all exceptions with a bare 'except Exception: pass', which could hide unexpected failures and make debugging difficult. Consider being more specific about what exceptions are expected, or remove the try/except if assertLogs will properly handle the case.

Suggested change
try:
balance_cbps.cbps(
sample_df,
sample_weights,
target_df,
target_weights,
transformations=None,
cbps_method="over",
)
except Exception:
pass
balance_cbps.cbps(
sample_df,
sample_weights,
target_df,
target_weights,
transformations=None,
cbps_method="over",
)

Copilot uses AI. Check for mistakes.
Comment on lines +1815 to +1825
try:
balance_cbps.cbps(
sample_df,
sample_weights,
target_df,
target_weights,
transformations=None,
cbps_method="over",
)
except Exception:
pass
Copy link

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test catches all exceptions with a bare 'except Exception: pass', which could hide unexpected failures and make debugging difficult. Consider being more specific about what exceptions are expected, or remove the try/except if assertLogs will properly handle the case.

Suggested change
try:
balance_cbps.cbps(
sample_df,
sample_weights,
target_df,
target_weights,
transformations=None,
cbps_method="over",
)
except Exception:
pass
balance_cbps.cbps(
sample_df,
sample_weights,
target_df,
target_weights,
transformations=None,
cbps_method="over",
)

Copilot uses AI. Check for mistakes.
Comment on lines +1586 to +1591
result.__getitem__ = lambda self, key: {
"success": np.bool_(False),
"message": "Maximum iterations reached",
"x": x0,
"fun": 100.0,
}[key]
Copy link

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lambda function for getitem captures a variable named 'self' which shadows the MagicMock instance. This works but is confusing. Consider using a different parameter name (e.g., 'obj' or '_') to avoid shadowing: result.__getitem__ = lambda obj, key: {...}[key]

Copilot uses AI. Check for mistakes.
Comment on lines +1521 to +1526
successful_result.__getitem__ = lambda self, key: {
"success": np.bool_(True),
"message": "Optimization terminated successfully",
"x": np.array([0.1, 0.1]),
"fun": 0.01,
}[key]
Copy link

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lambda function for getitem captures a variable named 'self' which shadows the MagicMock instance. This works but is confusing. Consider using a different parameter name (e.g., 'obj' or '_') to avoid shadowing throughout this test class.

Copilot uses AI. Check for mistakes.
"""Test seaborn_plot_dist uses plot_qq for numeric variables with qq dist_type.

Verifies line 714 in weighted_comparisons_plots.py.
"""
Copy link

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test uses np.random.randn without setting a seed, which could lead to non-deterministic test behavior. Consider adding np.random.seed before generating random data to ensure reproducibility and test stability.

Suggested change
"""
"""
np.random.seed(0)

Copilot uses AI. Check for mistakes.

from balance.stats_and_plots.weighted_comparisons_plots import plotly_plot_qq

df = pd.DataFrame({"v1": np.random.randn(50), "weight": np.ones(50)})
Copy link

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test uses np.random.randn without setting a seed, which could lead to non-deterministic test behavior. Consider adding np.random.seed before generating random data to ensure reproducibility and test stability.

Copilot uses AI. Check for mistakes.
"""
from balance.stats_and_plots.weighted_comparisons_plots import plotly_plot_dist

df = pd.DataFrame({"v1": np.random.randn(30), "weight": np.ones(30)})
Copy link

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test uses np.random.randn without setting a seed, which could lead to non-deterministic test behavior. Consider adding np.random.seed before generating random data to ensure reproducibility and test stability.

Copilot uses AI. Check for mistakes.
Comment on lines +1627 to +1628
self.assertTrue(
len(log_context.records) > 0,
Copy link

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assertTrue(a > b) cannot provide an informative message. Using assertGreater(a, b) instead will give more informative messages.

Suggested change
self.assertTrue(
len(log_context.records) > 0,
self.assertGreater(
len(log_context.records),
0,

Copilot uses AI. Check for mistakes.
self.assertTrue(os.path.isfile(diagnostics_out_file))

output_df = pd.read_csv(out_file)
self.assertTrue(len(output_df) > 0)
Copy link

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assertTrue(a > b) cannot provide an informative message. Using assertGreater(a, b) instead will give more informative messages.

Suggested change
self.assertTrue(len(output_df) > 0)
self.assertGreater(len(output_df), 0)

Copilot uses AI. Check for mistakes.
@talgalili
Copy link
Contributor Author

@copilot open a new pull request to apply changes based on the comments in this thread

talgalili added a commit to talgalili/balance that referenced this pull request Jan 19, 2026
Summary:

The coverage report showed 98% overall coverage with 7 files having gaps. These tests cover previously untested edge cases including:

- CLI exception handling paths for weighting failures
- Sample class design effect diagnostics and IPW model parameters
- CBPS optimization convergence warnings and constraint violation exceptions
- Plotting functions with missing values, default parameters, and various dist_types
- Distance metrics with empty numeric columns

Differential Revision: D90946146
Copilot AI review requested due to automatic review settings January 19, 2026 07:26
talgalili added a commit to talgalili/balance that referenced this pull request Jan 19, 2026
Summary:

The coverage report showed 98% overall coverage with 7 files having gaps. These tests cover previously untested edge cases including:

- CLI exception handling paths for weighting failures
- Sample class design effect diagnostics and IPW model parameters
- CBPS optimization convergence warnings and constraint violation exceptions
- Plotting functions with missing values, default parameters, and various dist_types
- Distance metrics with empty numeric columns

Differential Revision: D90946146
talgalili added a commit to talgalili/balance that referenced this pull request Jan 19, 2026
Summary:

The coverage report showed 98% overall coverage with 7 files having gaps. These tests cover previously untested edge cases including:

- CLI exception handling paths for weighting failures
- Sample class design effect diagnostics and IPW model parameters
- CBPS optimization convergence warnings and constraint violation exceptions
- Plotting functions with missing values, default parameters, and various dist_types
- Distance metrics with empty numeric columns

Differential Revision: D90946146
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comments suppressed due to low confidence (1)

tests/test_cbps.py:1824

  • 'except' clause does nothing but pass and there is no explanatory comment.

Comment on lines +1627 to +1630
def test_gmm_loss_bal_init_convergence_failure_warning(self) -> None:
"""Test line 765: Warning when gmm_loss with beta_balance fails.

Uses mocking to simulate the second gmm_loss optimization failing.
Copy link

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assertTrue(a > b) cannot provide an informative message. Using assertGreater(a, b) instead will give more informative messages.

Copilot uses AI. Check for mistakes.

def test_gmm_loss_glm_init_convergence_failure_warning(self) -> None:
"""Test line 747: Warning when gmm_loss with gmm_init fails.

Copy link

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'except' clause does nothing but pass and there is no explanatory comment.

Copilot uses AI. Check for mistakes.
or "convergence" in r.getMessage().lower()
]
self.assertTrue(
len(gmm_warnings) > 0 or len(log_context.records) > 0,
Copy link

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'except' clause does nothing but pass and there is no explanatory comment.

Copilot uses AI. Check for mistakes.
"success": np.bool_(True),
"message": "Success",
"x": np.array([1.0]),
}
Copy link

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'except' clause does nothing but pass and there is no explanatory comment.

Copilot uses AI. Check for mistakes.
talgalili added a commit to talgalili/balance that referenced this pull request Jan 19, 2026
Summary:

The coverage report showed 98% overall coverage with 7 files having gaps. These tests cover previously untested edge cases including:

- CLI exception handling paths for weighting failures
- Sample class design effect diagnostics and IPW model parameters
- CBPS optimization convergence warnings and constraint violation exceptions
- Plotting functions with missing values, default parameters, and various dist_types
- Distance metrics with empty numeric columns

Differential Revision: D90946146
talgalili added a commit to talgalili/balance that referenced this pull request Jan 19, 2026
Summary:

The coverage report showed 98% overall coverage with 7 files having gaps. These tests cover previously untested edge cases including:

- CLI exception handling paths for weighting failures
- Sample class design effect diagnostics and IPW model parameters
- CBPS optimization convergence warnings and constraint violation exceptions
- Plotting functions with missing values, default parameters, and various dist_types
- Distance metrics with empty numeric columns

Differential Revision: D90946146
Copilot AI review requested due to automatic review settings January 19, 2026 08:22
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment on lines +2314 to +2351


class TestRelativeFrequencyTableSeries(balance.testutil.BalanceTestCase):
"""Test cases for relative_frequency_table with Series (lines 677, 683)."""

def test_relative_frequency_table_with_series(self) -> None:
"""Test relative_frequency_table with Series input.

Verifies lines 676-683 in weighted_stats.py.
"""
from balance.stats_and_plots.weighted_stats import relative_frequency_table

s = pd.Series(["a", "a", "b", "c"])
result = relative_frequency_table(s)
self.assertIsInstance(result, pd.DataFrame)
self.assertIn("prop", result.columns)

def test_relative_frequency_table_with_unnamed_series(self) -> None:
"""Test relative_frequency_table with unnamed Series.

Verifies lines 679-680 in weighted_stats.py.
"""
from balance.stats_and_plots.weighted_stats import relative_frequency_table

s = pd.Series(["a", "a", "b"])
s.name = None
result = relative_frequency_table(s)
self.assertIn("group", result.columns)

def test_relative_frequency_table_raises_on_invalid_type(self) -> None:
"""Test relative_frequency_table raises on invalid input type.

Verifies lines 682-683 in weighted_stats.py.
"""
from balance.stats_and_plots.weighted_stats import relative_frequency_table

with self.assertRaises((TypeError, AttributeError)):
relative_frequency_table([1, 2, 3]) # type: ignore
Copy link

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test class duplicates the exact same tests already present in the TestDescriptiveStatsEdgeCases class (lines 2260-2293). The test_relative_frequency_table_with_series, test_relative_frequency_table_with_unnamed_series, and test_relative_frequency_table_raises_on_invalid_type tests are identical duplicates and should be removed to avoid test redundancy.

Suggested change
class TestRelativeFrequencyTableSeries(balance.testutil.BalanceTestCase):
"""Test cases for relative_frequency_table with Series (lines 677, 683)."""
def test_relative_frequency_table_with_series(self) -> None:
"""Test relative_frequency_table with Series input.
Verifies lines 676-683 in weighted_stats.py.
"""
from balance.stats_and_plots.weighted_stats import relative_frequency_table
s = pd.Series(["a", "a", "b", "c"])
result = relative_frequency_table(s)
self.assertIsInstance(result, pd.DataFrame)
self.assertIn("prop", result.columns)
def test_relative_frequency_table_with_unnamed_series(self) -> None:
"""Test relative_frequency_table with unnamed Series.
Verifies lines 679-680 in weighted_stats.py.
"""
from balance.stats_and_plots.weighted_stats import relative_frequency_table
s = pd.Series(["a", "a", "b"])
s.name = None
result = relative_frequency_table(s)
self.assertIn("group", result.columns)
def test_relative_frequency_table_raises_on_invalid_type(self) -> None:
"""Test relative_frequency_table raises on invalid input type.
Verifies lines 682-683 in weighted_stats.py.
"""
from balance.stats_and_plots.weighted_stats import relative_frequency_table
with self.assertRaises((TypeError, AttributeError)):
relative_frequency_table([1, 2, 3]) # type: ignore

Copilot uses AI. Check for mistakes.
Comment on lines +1514 to +1519
result.__getitem__ = lambda self, key: {
"success": np.bool_(False),
"message": "Did not converge to a solution satisfying the constraints",
"x": x0,
"fun": 100.0,
}[key]
Copy link

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lambda function used in mock_minimize has a closure issue. The lambda captures 'self' from the outer scope which shadows the 'self' parameter in the lambda. This can lead to unexpected behavior. Consider using a different approach, such as using result.configure_mock or setting attributes directly on the MagicMock object instead of overriding getitem with a lambda.

Copilot uses AI. Check for mistakes.
Comment on lines +1566 to +1579
result.__getitem__ = lambda self, key: {
"success": np.bool_(True),
"message": "Success",
"x": x0,
"fun": 1.0,
}[key]
else:
# Both GMM optimizations fail with constraint violation
result.__getitem__ = lambda self, key: {
"success": np.bool_(False),
"message": "Did not converge to a solution satisfying the constraints",
"x": x0,
"fun": 100.0,
}[key]
Copy link

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lambda function used in mock_minimize has a closure issue. The lambda captures 'self' from the outer scope which shadows the 'self' parameter in the lambda. This can lead to unexpected behavior. Consider using a different approach, such as using result.configure_mock or setting attributes directly on the MagicMock object instead of overriding getitem with a lambda.

Suggested change
result.__getitem__ = lambda self, key: {
"success": np.bool_(True),
"message": "Success",
"x": x0,
"fun": 1.0,
}[key]
else:
# Both GMM optimizations fail with constraint violation
result.__getitem__ = lambda self, key: {
"success": np.bool_(False),
"message": "Did not converge to a solution satisfying the constraints",
"x": x0,
"fun": 100.0,
}[key]
result_data = {
"success": np.bool_(True),
"message": "Success",
"x": x0,
"fun": 1.0,
}
result.__getitem__.side_effect = result_data.__getitem__
else:
# Both GMM optimizations fail with constraint violation
result_data = {
"success": np.bool_(False),
"message": "Did not converge to a solution satisfying the constraints",
"x": x0,
"fun": 100.0,
}
result.__getitem__.side_effect = result_data.__getitem__

Copilot uses AI. Check for mistakes.
Comment on lines +1712 to +1713
df1 = pd.DataFrame({"v1": np.random.randn(50)})
df2 = pd.DataFrame({"v1": np.random.randn(50)})
Copy link

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests should be deterministic and stable. Set a random seed before calling np.random.randn() to ensure reproducible test results. This aligns with the project's guideline to "avoid order/time dependence and uncontrolled randomness."

Suggested change
df1 = pd.DataFrame({"v1": np.random.randn(50)})
df2 = pd.DataFrame({"v1": np.random.randn(50)})
rng = np.random.RandomState(0)
df1 = pd.DataFrame({"v1": rng.randn(50)})
df2 = pd.DataFrame({"v1": rng.randn(50)})

Copilot uses AI. Check for mistakes.
talgalili added a commit to talgalili/balance that referenced this pull request Jan 19, 2026
Summary:

The coverage report showed 98% overall coverage with 7 files having gaps. These tests cover previously untested edge cases including:

- CLI exception handling paths for weighting failures
- Sample class design effect diagnostics and IPW model parameters
- CBPS optimization convergence warnings and constraint violation exceptions
- Plotting functions with missing values, default parameters, and various dist_types
- Distance metrics with empty numeric columns

Differential Revision: D90946146
talgalili added a commit to talgalili/balance that referenced this pull request Jan 19, 2026
Summary:

The coverage report showed 98% overall coverage with 7 files having gaps. These tests cover previously untested edge cases including:

- CLI exception handling paths for weighting failures
- Sample class design effect diagnostics and IPW model parameters
- CBPS optimization convergence warnings and constraint violation exceptions
- Plotting functions with missing values, default parameters, and various dist_types
- Distance metrics with empty numeric columns

Differential Revision: D90946146
Copilot AI review requested due to automatic review settings January 19, 2026 08:40
talgalili added a commit to talgalili/balance that referenced this pull request Jan 19, 2026
Summary:

The coverage report showed 98% overall coverage with 7 files having gaps. These tests cover previously untested edge cases including:

- CLI exception handling paths for weighting failures
- Sample class design effect diagnostics and IPW model parameters
- CBPS optimization convergence warnings and constraint violation exceptions
- Plotting functions with missing values, default parameters, and various dist_types
- Distance metrics with empty numeric columns

Differential Revision: D90946146
Copilot AI review requested due to automatic review settings January 19, 2026 12:11
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment on lines +1414 to +1415
self.assertTrue(
len(log_context.records) > 0,
Copy link

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assertTrue(a > b) cannot provide an informative message. Using assertGreater(a, b) instead will give more informative messages.

Suggested change
self.assertTrue(
len(log_context.records) > 0,
self.assertGreater(
len(log_context.records),
0,

Copilot uses AI. Check for mistakes.
talgalili added a commit to talgalili/balance that referenced this pull request Jan 19, 2026
Summary:

Adds comprehensive tests covering previously untested edge cases identified in the coverage report. The 7 files with coverage gaps now have tests for:

- **CLI**: Exception handling paths for weighting failures
- **Sample class**: Design effect diagnostics and IPW model parameters
- **CBPS**: Optimization convergence warnings and constraint violation exceptions
- **Plotting**: Functions with missing values, default parameters, and various dist_types
- **Distance metrics**: Empty numeric columns edge case

This improves the overall test coverage reliability and catches potential regressions in error handling paths.

Differential Revision: D90946146
talgalili added a commit to talgalili/balance that referenced this pull request Jan 19, 2026
Summary:

Adds comprehensive tests covering previously untested edge cases identified in the coverage report. The 7 files with coverage gaps now have tests for:

- **CLI**: Exception handling paths for weighting failures
- **Sample class**: Design effect diagnostics and IPW model parameters
- **CBPS**: Optimization convergence warnings and constraint violation exceptions
- **Plotting**: Functions with missing values, default parameters, and various dist_types
- **Distance metrics**: Empty numeric columns edge case

This improves the overall test coverage reliability and catches potential regressions in error handling paths.

Differential Revision: D90946146
Copilot AI review requested due to automatic review settings January 19, 2026 12:54
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

talgalili added a commit to talgalili/balance that referenced this pull request Jan 19, 2026
Summary:

Adds comprehensive tests covering previously untested edge cases identified in the coverage report. The 7 files with coverage gaps now have tests for:

- **CLI**: Exception handling paths for weighting failures
- **Sample class**: Design effect diagnostics and IPW model parameters
- **CBPS**: Optimization convergence warnings and constraint violation exceptions
- **Plotting**: Functions with missing values, default parameters, and various dist_types
- **Distance metrics**: Empty numeric columns edge case

This improves the overall test coverage reliability and catches potential regressions in error handling paths.

Differential Revision: D90946146
talgalili added a commit to talgalili/balance that referenced this pull request Jan 19, 2026
Summary:

Adds comprehensive tests covering previously untested edge cases identified in the coverage report. The 7 files with coverage gaps now have tests for:

- **CLI**: Exception handling paths for weighting failures
- **Sample class**: Design effect diagnostics and IPW model parameters
- **CBPS**: Optimization convergence warnings and constraint violation exceptions
- **Plotting**: Functions with missing values, default parameters, and various dist_types
- **Distance metrics**: Empty numeric columns edge case

This improves the overall test coverage reliability and catches potential regressions in error handling paths.

Differential Revision: D90946146
Copilot AI review requested due to automatic review settings January 19, 2026 13:37
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

talgalili added a commit to talgalili/balance that referenced this pull request Jan 19, 2026
Summary:

Adds comprehensive tests covering previously untested edge cases identified in the coverage report. The 7 files with coverage gaps now have tests for:

- **CLI**: Exception handling paths for weighting failures
- **Sample class**: Design effect diagnostics and IPW model parameters
- **CBPS**: Optimization convergence warnings and constraint violation exceptions
- **Plotting**: Functions with missing values, default parameters, and various dist_types
- **Distance metrics**: Empty numeric columns edge case

This improves the overall test coverage reliability and catches potential regressions in error handling paths.

Differential Revision: D90946146
Summary:

Adds comprehensive tests covering previously untested edge cases identified in the coverage report. The 7 files with coverage gaps now have tests for:

- **CLI**: Exception handling paths for weighting failures
- **Sample class**: Design effect diagnostics and IPW model parameters
- **CBPS**: Optimization convergence warnings and constraint violation exceptions
- **Plotting**: Functions with missing values, default parameters, and various dist_types
- **Distance metrics**: Empty numeric columns edge case

This improves the overall test coverage reliability and catches potential regressions in error handling paths.

Differential Revision: D90946146
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

@talgalili
Copy link
Contributor Author

FYI @neuralsorcerer we got to 100% test coverage!

image

@neuralsorcerer
Copy link
Collaborator

Thank you for the work, @talgalili now that we’ve reached 100% test coverage, what do you think about moving our core computations to Rust for better performance? We could use maturin + pyo3 bindings to compile into python package.

@talgalili
Copy link
Contributor Author

@neuralsorcerer I have no idea how that would 'play' with Meta's internal systems.
Could you please open a dedicated issue and give details of what you had in mind?

I have a longer roadmap in mind (roughly outlines in milestones 0.16 and above). But I need to get internal Meta alignment on it.
This could take a couple of weeks to complete.

If you are looking for a next step issue, please reach out via email and let's discuss ordering. 🙇

@meta-codesync
Copy link

meta-codesync bot commented Jan 19, 2026

This pull request has been merged in d0469bc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants