-
Notifications
You must be signed in to change notification settings - Fork 11
test: example from wiki #63
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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>
|
🐻 approved |
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.