Skip to content
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

[SPARK-23698] Define xrange() for Python 3 in dumpdata_script.py #21959

Closed
wants to merge 1 commit into from

Conversation

cclauss
Copy link

@cclauss cclauss commented Aug 2, 2018

xrange() was removed in Python 3 in favor of range(). This simple change resolves three Undefined Names was originally suggested in #20838 which is currently mired in 50+ comments on unrelated modifications. @HyukjinKwon @holdenk

What changes were proposed in this pull request?

(Please fill in changes proposed in this fix)

How was this patch tested?

(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)
(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)

Please review http://spark.apache.org/contributing.html before opening a pull request.

__xrange()__ was removed in Python 3 in favor of __range()__.  This simple change removes three Undefined Names was originally suggested in apache#20838 which is currently mired in 50+ comments on unrelated modifications.  @HyukjinKwon @holdenk
@holdensmagicalunicorn
Copy link

@cclauss, thanks! I am a bot who has found some folks who might be able to help with the review:@pwendell and @HyukjinKwon

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@HyukjinKwon
Copy link
Member

I think you can push the changes in #20838 and close this one. They look virtually the same issue.

@cclauss
Copy link
Author

cclauss commented Aug 2, 2018

As it says in the commit message, these changes are already in #20838 but that PR has been open for 139 days and has 50+ comments. The only way that I seem to make progress is by opening separate PRs that are easier to review and approve.

@HyukjinKwon
Copy link
Member

I am going to review and merge that one soon. It doesn't need to open multiple PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants