-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[PYTHON] Changes input variable to not conflict with built-in function #20775
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
Conversation
Signed-off-by: DylanGuedes <[email protected]>
|
ok to test |
| """ | ||
| An example of how to use DataFrame for ML. Run with:: | ||
| bin/spark-submit examples/src/main/python/ml/dataframe_example.py <input> | ||
| bin/spark-submit examples/src/main/python/ml/dataframe_example.py <dataset> |
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.
Shall we change dataset to like .. input_path or path? I think the name dataset is usually used for Dataset instance.
|
I think it's fine. It should better avoid shadowing builtin names but can you check out other examples too while we are here? |
|
Sure, and thanks for the review! I used the "dataset" name because I checked a few examples that also use it, but I'll rename to path then. I already checked all other examples using grep and this is the only one using "input" name. |
|
Test build #88124 has finished for PR 20775 at commit
|
Signed-off-by: DylanGuedes <[email protected]>
|
@HyukjinKwon I checked every other built in function and this was the only one being used. What you think? |
|
Test build #88125 has finished for PR 20775 at commit
|
HyukjinKwon
left a comment
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
| # Load input data | ||
| print("Loading LIBSVM file with UDT from " + input + ".") | ||
| df = spark.read.format("libsvm").load(input).cache() | ||
| # Load file from path |
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.
Let's just write it as Load an input file.
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.
Done.
Signed-off-by: DylanGuedes <[email protected]>
|
Test build #88128 has finished for PR 20775 at commit
|
felixcheung
left a comment
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.
can you update the PR title to say [PYTHON] example?
|
Sure. Done. |
|
Merged to master and branch-2.3. |
Signed-off-by: DylanGuedes <djmgguedesgmail.com> ## What changes were proposed in this pull request? Changes variable name conflict: [input is a built-in python function](https://stackoverflow.com/questions/20670732/is-input-a-keyword-in-python). ## How was this patch tested? I runned the example and it works fine. Author: DylanGuedes <[email protected]> Closes #20775 from DylanGuedes/input_variable. (cherry picked from commit b6f837c) Signed-off-by: hyukjinkwon <[email protected]>
Signed-off-by: DylanGuedes <djmgguedesgmail.com> ## What changes were proposed in this pull request? Changes variable name conflict: [input is a built-in python function](https://stackoverflow.com/questions/20670732/is-input-a-keyword-in-python). ## How was this patch tested? I runned the example and it works fine. Author: DylanGuedes <[email protected]> Closes apache#20775 from DylanGuedes/input_variable. (cherry picked from commit b6f837c) Signed-off-by: hyukjinkwon <[email protected]>
Signed-off-by: DylanGuedes [email protected]
What changes were proposed in this pull request?
Changes variable name conflict: input is a built-in python function.
How was this patch tested?
I runned the example and it works fine.