-
Notifications
You must be signed in to change notification settings - Fork 12
gh-961: use rng_dispatcher throughout code
#962
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
Co-authored-by: Connor Aird <c.aird@ucl.ac.uk>
| Use `urng` for array API tests. | ||
| """ | ||
| return np.random.default_rng(seed=SEED) | ||
| return np.random.default_rng(seed=42) |
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.
Can we not use rng_dispatcher here?
| return np.random.default_rng(seed=42) | |
| return _utils.rng_dispatcher(xp=np) |
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.
So we can, but then the typing becomes UnfiedGenerator rather than np.random.Generator.
Either:
- Doesn't matter and use dispatcher
- Suppress mypy error
What do you think?
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.
Hmm I see. I think I like having the correct type. Could we reuse the seed definition?
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.
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.
So I think let's leave this for now, and fix it once #971 is merged.
Description
The helper function isn't used much and there is duplication. This PR addresses that and also means we don't have to specify the seed in as many places.
Closes: #961
Checks