-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
[BEAM-1251] print() is a function in Python 3 #6166
Conversation
There seem to be two more occurences
|
Thanks @superbobry, those instances were in comments, not in live code but I have converted them for completeness. |
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.
LGTM!
@charlesccychen @holdenk Your reviews please? |
@pabloem Your review please? |
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! This LGTM.
@holdenk This is merged now but I am wondering how the testing did not catch this and how we can prevent similar regression in the future. https://github.com/apache/beam/blob/master/sdks/python/scripts/run_mini_py3lint.sh#L51 should be able to spot these as syntax errors but perhaps it is only look at a portion of the codebase instead of looking at all Python files in the repo. On Travis, it takes 20 sec to do this flake8 test on the entire repo. |
Line 110 in f31b789
I think the tools did not catch it because the occurrences are in the comment. |
The first three files contained print statements in live code. These files are in .test-infra/jenkins which flake8 does not currently test. |
print() is a function in Python 3 # just like #4784, #5531, and #5890
@charlesccychen @holdenk @superbobry
Please add a meaningful description for your change here
Follow this checklist to help us incorporate your contribution quickly and easily:
[BEAM-XXX] Fixes bug in ApproximateQuantiles
, where you replaceBEAM-XXX
with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.It will help us expedite review of your Pull Request if you tag someone (e.g.
@username
) to look at it.Post-Commit Tests Status (on master branch)