-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Remove seeded_test fixture and migrate tests to numpy Generator API #8037
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
base: main
Are you sure you want to change the base?
Remove seeded_test fixture and migrate tests to numpy Generator API #8037
Conversation
tests/conftest.py
Outdated
| @pytest.fixture(scope="function", autouse=False) | ||
| def seeded_test(): | ||
| np.random.seed(20160911) | ||
| @pytest.fixture(scope="function") |
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.
I rather move away from magic fixtures, just let each test create its own rng, it's not that much extra code
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.
Okay, I will make required changes
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.
Please check the changes
ricardoV94
left a comment
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.
Looks great!
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8037 +/- ##
==========================================
+ Coverage 91.44% 91.47% +0.02%
==========================================
Files 116 116
Lines 19002 19065 +63
==========================================
+ Hits 17377 17440 +63
Misses 1625 1625 🚀 New features to boost your workflow:
|
|
For last commit I prefer the more strict old test that no draws are close. Can you use a different seed and revert to the old check? |
|
I will look into it. |
|
Thanks for the suggestion! |
|
need to run pre-commit |
Utkarshkarki
left a comment
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.
yes i will do it
Utkarshkarki
left a comment
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.
I think all the changes are perfect now, and I have reviewed them. Please tell if any more changes are nedded.
ricardoV94
left a comment
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.
3 tests still have a changed condition that is less strict than before
|
Please, can you review the changes once |
Migrate from seeded_test fixture to modern numpy Generator API
Replaces the legacy
seeded_testfixture usingnp.random.seed()with a modern rng fixture usingnp.random.default_rng(). This eliminates global state pollution and enables thread-safe random number generation.Changes:
seeded_testwith rng fixture in conftest.pynp.random.xxx()calls torng.xxx()using Generator APIBenefits:
Closes #8035