-
Notifications
You must be signed in to change notification settings - Fork 976
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
Fix batch_sampler maybe None error #3025
Conversation
For more details, see: huggingface#3011
Thanks a lot for creating the PR. If you could also add a unit test to |
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.
agreed with Benjamin, let's add a test here then ✅ , thanks!
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
Add unit test for dataloader with batch_size=None when using Iterabledataset
The unit test code is added, please help to review and merge. |
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.
Thanks! Overall this looks good to me, please run pip install -e .[quality]; make style; make quality;
to fix the quality failures, and I made a comment for why the tests are failing
Co-authored-by: Zach Mueller <[email protected]>
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.
Thanks for adding the test @candlewill. This PR is almost good to go, only the linter needs to be satisfied. This is because these lines are indented one level too far.
Fix inconsistent indentation
I am sure now everything is OK, please help review again. |
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.
Thanks for adding a test!
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.
Thanks, LGTM.
Fix batch_sampler maybe None error
Fixes # (issue)
#3011
Before submitting
Pull Request section?
to it if that's the case.
documentation guidelines, and
here are tips on formatting docstrings.
Who can review?
Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.