Merged
Conversation
Signed-off-by: Paul Zabelin <paulzabelin@artium.ai>
Signed-off-by: Paul Zabelin <paulzabelin@artium.ai>
Signed-off-by: Paul Zabelin <paulzabelin@artium.ai>
d66d649 to
879b9e8
Compare
Contributor
There was a problem hiding this comment.
Pull Request Overview
A refactoring to correct statistical calculations in calculate_ztest and improve test clarity for sample size functions.
- Corrected the parameters passed to proportions_ztest in calculate_ztest.
- Renamed and introduced new sample size functions to handle both one failure and no failure cases with accompanying tests.
- Improved error messages and added a test case based on a wiki example.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| examples/team_recommender/tests/test_helpers.py | Refactored sample size helper functions and updated tests with consistent naming. |
| examples/team_recommender/tests/test_proportions_ztest.py | Corrected calculate_ztest parameters and updated significance tests with a wiki example. |
Comments suppressed due to low confidence (3)
examples/team_recommender/tests/test_proportions_ztest.py:126
- The error message does not match the test parameters; it refers to '0 out of 3' while the sample size is 1000. Please update the message to accurately reflect the test scenario.
assert is_statistically_significant(0.7, 0, 1000), "not significant result for 0 out of 3"
examples/team_recommender/tests/test_proportions_ztest.py:131
- The assertion expects a statistically significant result, but the error message suggests otherwise. Please align the error message with the expected outcome.
assert is_statistically_significant(0.7, 0, 10), "no improvement detected at 10"
examples/team_recommender/tests/test_proportions_ztest.py:139
- The error message implies a lack of improvement despite the assertion requiring statistical significance. Please update the message to match the intended check.
assert is_statistically_significant(0.97, 0, 100), "no improvement detected at 100"
Signed-off-by: Paul Zabelin <paulzabelin@artium.ai>
Signed-off-by: Paul Zabelin <paulzabelin@artium.ai>
test_no_failures_always_cause_insignificance Signed-off-by: Paul Zabelin <paulzabelin@artium.ai>
Signed-off-by: Paul Zabelin <paulzabelin@artium.ai>
as they generate stats only for 95% confidence and we use 90% Signed-off-by: Paul Zabelin <paulzabelin@artium.ai>
Contributor
|
🐻 approved |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request includes several changes to the
examples/team_recommender/testssuite, focusing on statistical significance testing and sample size calculations. The most important changes include the addition of new functions, modifications to existing test functions, and the removal of a test file.New functions and modifications:
examples/team_recommender/tests/helpers.py: Added a new functionis_statistically_significantto determine statistical significance.Test function updates:
examples/team_recommender/tests/test_helpers.py: Updatedtest_next_success_rateto usepytest.approxfor better precision in assertions.examples/team_recommender/tests/test_helpers.py: Added parameterized tests fortest_next_sample_size_with_1_failureandtest_next_no_failure_sample_size_via_loopto cover more cases.examples/team_recommender/tests/test_helpers.py: Added a new testtest_example_on_wikito validate statistical significance and sample size calculations.File removals and dependency updates:
examples/team_recommender/tests/test_proportions_ztest.py: Removed the entire file as it is no longer needed.pyproject.toml: Removed thestatsmodelsdependency, which was used in the deleted test file.