Update core.py [If funnction do not return any thing then warning will be pop up] #28680
Conversation
If "process" fun returns not return any thing then warning will be pop up
Codecov Report
@@ Coverage Diff @@
## master #28680 +/- ##
=======================================
Coverage 71.96% 71.97%
=======================================
Files 680 680
Lines 100163 100171 +8
=======================================
+ Hits 72087 72094 +7
- Misses 26498 26499 +1
Partials 1578 1578
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 4 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment |
|
assign set of reviewers |
|
Assigning reviewers. If you would like to opt out of this review, comment R: @jrmccluskey for label python. Available commands:
The PR bot will only process comments in the main thread (not review comments). |
| elif _check_fn_use_yield_and_return(self.fn.process) is None: | ||
| _LOGGER.warning( | ||
| 'no iterator is returned by the process method in %s', | ||
| self.fn.__class__) | ||
|
|
There was a problem hiding this comment.
Mechanically you probably don't want to re-run _check_fn_use_yield_and_return() here. Consider capturing the output value the first time and using that here instead
| for line in source_code.split("\n"): | ||
| if line.lstrip().startswith("yield ") or line.lstrip().startswith( | ||
| "yield("): | ||
| has_yield = True | ||
| if line.lstrip().startswith("return ") or line.lstrip().startswith( | ||
| "return("): | ||
| has_return = True | ||
| if line.lstrip().startswith("return None"): |
There was a problem hiding this comment.
The warn could also be placed here instead, eliminating the need for the elif block.
| if len(source_code)==0: | ||
| return None |
There was a problem hiding this comment.
What is the purpose of this check?
|
I think this is trying to do the same thing as #28159 which was opened first and seems slightly more complete, so I think we should proceed with that one and I'm going to close this pull request. In general, please make sure that you are assigned to an issue before you start working on it to avoid duplicate work. If someone else is assigned but you're not sure if they're working on the problem, its usually best to ask first. |
If
self.fn.processdoes not return anything [None] thenwarningwill be popped aswhere changes are committed in function
_check_fn_use_yield_and_returnto check if yield or return any one is present in codethis Issue is related to #28061
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>instead.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.