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-35143][ML] Simplify Python for loop code #32246

Closed
wants to merge 11 commits into from

Conversation

ChenDou2021
Copy link

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Copy link
Member

@HyukjinKwon HyukjinKwon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I mentioned earlier in another PR, I don't think this is worthwhile. It's more about style preference.

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@ChenDou2021
Copy link
Author

As I mentioned earlier in another PR, I don't think this is worthwhile. It's more about style preference.

The file util.py also uses the dictionary + for loop generation at the bottom, and I think it can be simplified in the same way.
eg:uidMap = {stage.uid: stage for stage in nestedStages}

@Yikun
Copy link
Member

Yikun commented Apr 21, 2021

@ChenDou2021 Hi, thanks for the contributions, as mentioned from @HyukjinKwon , the changes in your patch are all coding style preference (also, there were some pros and cons discussion on list comprehes, such as link), so these changes would not be considered to be merged into main line code.

Btw, the failing test is due to the branch name is branch-3.2, the fork build test was filtered by


and the notify test can't find the job in your fork repo. If you have futher PR to submit, you should consider rename your branch to something like (SPARK-XXX or other name except branch-XXX).

@ChenDou2021
Copy link
Author

ChenDou2021 commented Apr 21, 2021 via email

@Yikun
Copy link
Member

Yikun commented Apr 21, 2021

@ChenDou2021 ok,I know,thanks

And much thanks your first step in Spark community, feel free to open the next PR if you find some other improvements.

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