Skip to content
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

Follow up making Python tests more deterministic #17272

Open
wants to merge 5 commits into
base: branch-24.12
Choose a base branch
from

Conversation

mroeschke
Copy link
Contributor

Description

Addressing comments in https://github.com/rapidsai/cudf/pull/17008/files#r1823318321 and https://github.com/rapidsai/cudf/pull/17008/files#r1823318898

Didn't touch the _fuzz_testing directory because maybe we don't want that to be deterministic?

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@mroeschke mroeschke added Python Affects Python cuDF API. improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Nov 7, 2024
@mroeschke mroeschke self-assigned this Nov 7, 2024
@mroeschke mroeschke requested review from a team as code owners November 7, 2024 18:32
Copy link
Contributor

@vyasr vyasr left a comment

Choose a reason for hiding this comment

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

One suggestion for improving the future usage but it's not a blocker if we decide not to go with it (I'm not sure if we can actually do what I suggest yet).

# Check for usage of default_rng without seeding
default_rng\(\)|
# Check for usage of np.random.seed
np.random.seed\(
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we keep this without the parenthesis to forbid usage of np.random.seed in favor of the newer default_rng API?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah interesting, NPY002 allows the np.random.seed callable to exist but complains when it's called. Sure I'll add this back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvement / enhancement to an existing function non-breaking Non-breaking change Python Affects Python cuDF API.
Projects
Status: In Progress
Status: In Progress
Development

Successfully merging this pull request may close these issues.

5 participants