Skip to content

Conversation

@paddyroddy
Copy link
Member

@paddyroddy paddyroddy commented Jan 12, 2026

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

  • Is your code passing linting?
  • Is your code passing tests?
  • Have you added additional tests (if required)?
  • Have you modified/extended the documentation (if required)?
  • Have you added a one-liner changelog entry above (if required)?

@paddyroddy paddyroddy self-assigned this Jan 12, 2026
@paddyroddy paddyroddy added enhancement New feature or request maintenance Maintenance: refactoring, typos, etc. labels Jan 12, 2026
@paddyroddy paddyroddy requested a review from connoraird January 12, 2026 17:48
Use `urng` for array API tests.
"""
return np.random.default_rng(seed=SEED)
return np.random.default_rng(seed=42)
Copy link
Contributor

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?

Suggested change
return np.random.default_rng(seed=42)
return _utils.rng_dispatcher(xp=np)

Copy link
Member Author

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?

Copy link
Contributor

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?

Copy link
Contributor

Choose a reason for hiding this comment

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

To solve this we decided to create #970. However, this is now blocked due to #972. Therefore, this can be merged as is for now.

Copy link
Member Author

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.

Base automatically changed from paddy/issue-958 to main January 14, 2026 17:56
@paddyroddy paddyroddy merged commit 8f4bb6b into main Jan 15, 2026
21 of 22 checks passed
@paddyroddy paddyroddy deleted the paddy/issue-961 branch January 15, 2026 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request maintenance Maintenance: refactoring, typos, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use rng_dispatcher where possible

3 participants