Generator support in ExternalSource #1832
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Michał Zientkiewicz <mzient@gmail.com>
Signed-off-by: Michał Zientkiewicz <mzient@gmail.com>
|
!build |
|
CI MESSAGE: [1206920]: BUILD STARTED |
szalpal
reviewed
Mar 23, 2020
Comment on lines
162
to
166
| If `True`, the source will be wrapped. Otherwise, StopIteration error wil be raised | ||
| when end of data is reached. This flag requires that `source` is either a collection, i.e. an | ||
| iteratble object where ``iter(source)`` will return a fresh iterator on each call or a | ||
| generator function. In the latter case, the generator function will be called again when more | ||
| data is requried than was yielded by the function. |
There was a problem hiding this comment.
Suggested change
| If `True`, the source will be wrapped. Otherwise, StopIteration error wil be raised | |
| when end of data is reached. This flag requires that `source` is either a collection, i.e. an | |
| iteratble object where ``iter(source)`` will return a fresh iterator on each call or a | |
| generator function. In the latter case, the generator function will be called again when more | |
| data is requried than was yielded by the function. | |
| If ``True``, the source will be wrapped. Otherwise, ``StopIteration`` will be raised | |
| when end of data is reached. This flag requires that ``source`` is either a collection, i.e. an | |
| iterable object where ``iter(source)`` will return a fresh iterator on each call, or a | |
| generator function. In the latter case, the generator function will be called again when more | |
| data is requested than was yielded by the function. |
My suggestions:
- I'm not sure if we should call
StopIteration"error", since it's regular python flow-control mechanism - typo in
wil - typo in
iteratble requried->requested- I'd wrap argument names etc. in double-back-ticks. It renders better, IMHO
szalpal
reviewed
Mar 23, 2020
| if cycle is not None: | ||
| raise ValueError("The argument `cycle` can only be specified if `source` is iterable") | ||
| if not callable(source): | ||
| raise TypeError("Source must be iterable or callable") | ||
| raise TypeError("Source must be callable, iterable or a perameterless generator function") |
There was a problem hiding this comment.
Suggested change
| raise TypeError("Source must be callable, iterable or a perameterless generator function") | |
| raise TypeError("Source must be callable, iterable or a parameterless generator function") |
szalpal
approved these changes
Mar 23, 2020
JanuszL
reviewed
Mar 23, 2020
| def _is_generator_function(x): | ||
| """Checks whether x is a generator function or a callable object | ||
| where __call__ is a generator function""" | ||
| import inspect |
There was a problem hiding this comment.
Why can't we import it globally?
JanuszL
reviewed
Mar 23, 2020
| def _get_callback_from_source(source, cycle): | ||
| iterable = False | ||
| if source is not None: | ||
| import inspect |
JanuszL
reviewed
Mar 23, 2020
| pipe.set_outputs(fn.external_source(gen, cycle = True)) | ||
| pipe.build() | ||
|
|
||
| for cycle in range(3): |
There was a problem hiding this comment.
You can add a negative test for a generator and cycle set to True.
mzient
force-pushed
the
ExternalSourceGenerator
branch
from
3829127
to
08dfccb
Compare
Signed-off-by: Michał Zientkiewicz <mzient@gmail.com>
mzient
force-pushed
the
ExternalSourceGenerator
branch
from
08dfccb
to
7371412
Compare
|
!build |
|
CI MESSAGE: [1207039]: BUILD FAILED |
JanuszL
approved these changes
Mar 23, 2020
|
CI MESSAGE: [1207088]: BUILD STARTED |
|
CI MESSAGE: [1207088]: BUILD PASSED |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why we need this PR?
Pick one, remove the rest
What happened in this PR?
Fill relevant points, put NA otherwise. Replace anything inside []
_get_callback_from_source- see if there's some use case not handled or badly handledJIRA TASK: N/A