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.

Sign up for GitHub

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

Jump to bottom

adds split_dataset utility #16398

Merged
merged 24 commits into from
May 9, 2022
Merged

adds split_dataset utility #16398

merged 24 commits into from
May 9, 2022

Conversation

prakashsellathurai
Copy link
Contributor

mentioned in #16394

@prakashsellathurai prakashsellathurai marked this pull request as ready for review April 11, 2022 13:46
@prakashsellathurai prakashsellathurai marked this pull request as draft April 11, 2022 14:57
@prakashsellathurai prakashsellathurai marked this pull request as draft April 11, 2022 14:57
@gbaned gbaned requested a review from fchollet April 11, 2022 15:13
@google-ml-butler google-ml-butler bot added the keras-team-review-pending Pending review by a Keras team member. label Apr 11, 2022
@gbaned gbaned removed the keras-team-review-pending Pending review by a Keras team member. label Apr 11, 2022
fchollet
fchollet reviewed Apr 11, 2022
View reviewed changes
Copy link
Member

@fchollet fchollet left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

Adding unit tests will help you make the use cases more concrete.

keras/utils/dataset_utils.py Outdated Show resolved Hide resolved
keras/utils/dataset_utils.py Outdated Show resolved Hide resolved
keras/utils/dataset_utils.py Outdated Show resolved Hide resolved
keras/utils/dataset_utils.py Outdated Show resolved Hide resolved
keras/utils/dataset_utils.py Outdated Show resolved Hide resolved
keras/utils/dataset_utils.py Outdated Show resolved Hide resolved
keras/utils/dataset_utils.py Outdated Show resolved Hide resolved
@fchollet fchollet mentioned this pull request Apr 13, 2022
Closed
fchollet
fchollet reviewed Apr 16, 2022
View reviewed changes
Copy link
Member

@fchollet fchollet left a comment

Choose a reason for hiding this comment

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

Thanks for the update!

keras/utils/dataset_utils.py Outdated Show resolved Hide resolved
keras/utils/dataset_utils.py Outdated Show resolved Hide resolved
keras/utils/dataset_utils_test.py Outdated Show resolved Hide resolved
keras/utils/dataset_utils_test.py Outdated Show resolved Hide resolved
keras/utils/dataset_utils_test.py Show resolved Hide resolved
keras/utils/dataset_utils.py Outdated Show resolved Hide resolved
keras/utils/dataset_utils.py Outdated Show resolved Hide resolved
keras/utils/dataset_utils_test.py Outdated Show resolved Hide resolved
keras/utils/dataset_utils.py Outdated Show resolved Hide resolved
@fchollet
Copy link
Member

Please let me know when you'd like me to review (and you can also graduate the PR from draft to ready).

@google-ml-butler google-ml-butler bot added the keras-team-review-pending Pending review by a Keras team member. label Apr 19, 2022
@gbaned gbaned requested review from fchollet and removed request for fchollet April 20, 2022 09:24
@prakashsellathurai
Copy link
Contributor Author

@fchollet I Incorporated the requested changes. The code passes both Pylint and Bazel tests. The pull request is now ready for review.

@gbaned gbaned requested review from fchollet and removed request for fchollet April 21, 2022 15:11
@rchao rchao added keras-team-review-pending Pending review by a Keras team member. and removed keras-team-review-pending Pending review by a Keras team member. labels Apr 21, 2022
fchollet
fchollet reviewed Apr 22, 2022
View reviewed changes
keras/utils/dataset_utils.py Outdated Show resolved Hide resolved
keras/utils/dataset_utils.py Outdated Show resolved Hide resolved
keras/utils/dataset_utils.py Outdated Show resolved Hide resolved
keras/utils/dataset_utils.py Outdated Show resolved Hide resolved
keras/utils/dataset_utils_test.py Outdated Show resolved Hide resolved
keras/utils/dataset_utils.py Outdated Show resolved Hide resolved
keras/utils/dataset_utils.py Outdated Show resolved Hide resolved
keras/utils/dataset_utils.py Outdated Show resolved Hide resolved
keras/utils/dataset_utils.py Outdated Show resolved Hide resolved
fchollet
fchollet approved these changes Apr 28, 2022
View reviewed changes
Copy link
Member

@fchollet fchollet left a comment

Choose a reason for hiding this comment

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

Excellent, I think we're good! Thanks for the update.

I added a comment but we can probably just fix it in post.

keras/utils/dataset_utils.py Outdated Show resolved Hide resolved
@google-ml-butler google-ml-butler bot added kokoro:force-run ready to pull Ready to be merged into the codebase labels Apr 28, 2022
@fchollet fchollet removed the keras-team-review-pending Pending review by a Keras team member. label Apr 28, 2022
@gbaned gbaned removed the ready to pull Ready to be merged into the codebase label May 2, 2022
@google-ml-butler google-ml-butler bot added kokoro:force-run ready to pull Ready to be merged into the codebase labels May 5, 2022
@copybara-service copybara-service bot merged commit 81c83b3 into keras-team:master May 9, 2022
@ricardoprins ricardoprins mentioned this pull request Jun 27, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fchollet fchollet fchollet approved these changes

Assignees

@gbaned gbaned

Labels
ready to pull Ready to be merged into the codebase size:M
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@prakashsellathurai @fchollet @rchao @kokoro-team @gbaned