-
Notifications
You must be signed in to change notification settings - Fork 3.6k
[Bug] Add SharedCycleIteratorState #8889
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #8889 +/- ##
=======================================
- Coverage 93% 89% -4%
=======================================
Files 169 172 +3
Lines 14079 14178 +99
=======================================
- Hits 13053 12568 -485
- Misses 1026 1610 +584 |
| @property | ||
| def done(self) -> bool: | ||
| if not self.has_reset: | ||
| raise MisconfigurationException("Please, call reset once all dataloaders have been added.") |
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.
| raise MisconfigurationException("Please, call reset once all dataloaders have been added.") | |
| raise MisconfigurationException("Please call reset once all dataloaders have been added.") |
with the comma in there it sounds a bit passive-aggressive 🤣
| class SharedCycleIteratorState: | ||
|
|
||
| mode: str = "max_size_cycle" |
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.
| class SharedCycleIteratorState: | |
| mode: str = "max_size_cycle" | |
| class SharedCycleIteratorState: | |
| """A state shared between all CylceIterators in a CombinedLoader. | |
| With a shared state, the iterators can decide to terminate based on the state of all others. | |
| If the mode is *max_size_cycle*, all iterators need to have finished before the combined loading is considered | |
| finished, and otherwise any iterator finishing early will lead to all iterators ending early. | |
| """ | |
| mode: str = "max_size_cycle" |
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.
What do you think of these docs? Did I get it right?
What does this PR do?
The following script will create an infinite loop on master.
This PR adds a
SharedCycleIteratorStateto synchornize the CycleIterator and stop on max_length.Fixes #<issue_number>
Does your PR introduce any breaking changes? If yes, please list them.
No.
Before submitting
PR review
Anyone in the community is welcome to review the PR.
Before you start reviewing make sure you have read Review guidelines. In short, see the following bullet-list:
Did you have fun?
Make sure you had fun coding 🙃