📝docs: clarify shuffle behavior and example in PyDataset #21847
Conversation
Summary of ChangesHello @maitry63, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the clarity and completeness of the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the documentation for the PyDataset class to clarify the behavior of the shuffle argument. The changes include adding documentation for the argument in the class docstring, providing a note on its usage, and updating the example to demonstrate shuffling. My review focuses on improving the clarity and correctness of the new documentation and example code. I've pointed out some grammatical and formatting issues in the docstring, a critical typo and a style issue in the example's __init__ method, and a logical flaw in the example's shuffling implementation which makes it non-functional.
| shuffle: Whether to shuffle the sample ordering at the end of | ||
| each epoch.This argument passed to `model.fit()`. when | ||
| `model.fit(.., shuffle=True)`, the training loop | ||
| automatically calls `on_epoch_end()` at each epoch | ||
| boundary, allowing datasets to implement custom | ||
| shuffling logic. Defaults to False. |
There was a problem hiding this comment.
There are a few grammatical and formatting issues in the docstring for the shuffle argument. It would be clearer with some corrections for spacing, sentence structure, and consistent code formatting.
| shuffle: Whether to shuffle the sample ordering at the end of | |
| each epoch.This argument passed to `model.fit()`. when | |
| `model.fit(.., shuffle=True)`, the training loop | |
| automatically calls `on_epoch_end()` at each epoch | |
| boundary, allowing datasets to implement custom | |
| shuffling logic. Defaults to False. | |
| shuffle: Whether to shuffle the sample ordering at the end of | |
| each epoch. This argument is passed to `model.fit()`. When | |
| `model.fit(..., shuffle=True)`, the training loop | |
| automatically calls `on_epoch_end()` at each epoch | |
| boundary, allowing datasets to implement custom | |
| shuffling logic. Defaults to `False`. |
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #21847 +/- ##
==========================================
- Coverage 82.66% 82.48% -0.19%
==========================================
Files 577 577
Lines 59506 59507 +1
Branches 9330 9331 +1
==========================================
- Hits 49193 49085 -108
- Misses 7910 8010 +100
- Partials 2403 2412 +9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| self.x, self.y = x_set, y_set | ||
| self.batch_size = batch_size | ||
| self.shuffle = shuffle | ||
| self.indices = np.arange(len(self.x)) |
There was a problem hiding this comment.
In the example, also do np.random.shuffle(self.indices) here when shuffle is True, since the best practice when shuffling is to do it for every epoch, not just epoch >= 1
| your dataset. Defaults to 10. | ||
| shuffle: Whether to shuffle the sample ordering at the end of | ||
| each epoch. This argument is passed to `model.fit()`. When | ||
| `model.fit(..., shuffle=True)`, the training loop |
There was a problem hiding this comment.
Add verb: "When calling"
4 participants
This PR updates the
PyDatasetclass docstring insidekeras/src/trainers/data_adapters/py_dataset_adapter.pyto include the documentation for the `shuffle´ argument.Context
Addresses issue #20142.
Notes