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
Rename shuffle to shuffle_method in remaining methods #10797
Rename shuffle to shuffle_method in remaining methods #10797
Conversation
Unit Test ResultsSee test report for an extended history of previous test failures. This is useful for diagnosing flaky tests. 15 files ± 0 15 suites ±0 3h 39m 43s ⏱️ - 5m 21s For more details on these failures, see this check. Results for commit 965dab5. ± Comparison against base commit 91dd425. This pull request removes 16 and adds 42 tests. Note that renamed tests count towards both.
♻️ This comment has been updated with latest results. |
@@ -209,9 +209,9 @@ def _determine_split_out_shuffle(shuffle, split_out): | |||
return config.get("dataframe.shuffle.method", None) or "tasks" | |||
else: | |||
return False | |||
if shuffle is True: | |||
if shuffle_method is True: |
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.
That is something we want to deprecate as well, shuffle_method can only be a string the determines the shuffle method
split_out should be used to signal if you want to shuffle or not
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.
We can do this as a follow up though
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.
Ya, if it's good for you, would like to do this in a follow up.
thx |
pre-commit run --all-files
Is there suppose to be a deprecation period before this renaming?