-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[SPARK-15761] [MLlib] [PySpark] Load ipython when default python is Python3 #13503
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
|
LGTM |
|
Test build #59970 has finished for PR 13503 at commit
|
|
cc @JoshRosen |
|
Merge? |
|
bump? |
bin/pyspark
Outdated
| # Determine the Python executable to use for the executors: | ||
| if [[ -z "$PYSPARK_PYTHON" ]]; then | ||
| if [[ $PYSPARK_DRIVER_PYTHON == *ipython* && $DEFAULT_PYTHON != "python2.7" ]]; then | ||
| if [[ $PYSPARK_DRIVER_PYTHON == *ipython* && $DEFAULT_PYTHON < "python2.7" ]]; then |
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.
If the default Python executable is named python but actually points to a Python 2.7+ or Python 3 executable then this is going to return a false positive because the string python is < python2.7.
Rather than relying on the name of the executable, it might make more sense to shell out to Python and ask it its version. For example, I think the following will work:
$DEFAULT_PYTHON -c 'import os; import sys; sys.exit(int(sys.version_info <= (2, 7, 0)))'If the Python version is greater than or equal to 2.7.0, then this will exit with 0; otherwise, this will exit with 1. This lets you check the exit status and not rely on the executable name.
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.
That said, the code in this PR is a strict improvement over the old overly-stringent check, so it's okay to merge it in its current form but I'd prefer to just make this super robust so we fix this once and for all.
|
@JoshRosen fixed, thanks! let me know if you need any other changes. |
|
I would also like to change if hash python2.7 2>/dev/null; then
# Attempt to use Python 2.7, if installed:
DEFAULT_PYTHON="python2.7"
else
DEFAULT_PYTHON="python"
fito just DEFAULT_PYTHON="python"I'm not sure if it is a great assumption that python2.7 is used by default, when python points to something else. |
|
lgtm |
|
Test build #61578 has finished for PR 13503 at commit
|
|
retest this please |
|
Test build #61592 has finished for PR 13503 at commit
|
|
Merged to master/2.0/1.6 |
…hon3 ## What changes were proposed in this pull request? I would like to use IPython with Python 3.5. It is annoying when it fails with IPython requires Python 2.7+; please install python2.7 or set PYSPARK_PYTHON when I have a version greater than 2.7 ## How was this patch tested It now works with IPython and Python3 Author: MechCoder <[email protected]> Closes #13503 from MechCoder/spark-15761. (cherry picked from commit 66283ee) Signed-off-by: Sean Owen <[email protected]>
…hon3 ## What changes were proposed in this pull request? I would like to use IPython with Python 3.5. It is annoying when it fails with IPython requires Python 2.7+; please install python2.7 or set PYSPARK_PYTHON when I have a version greater than 2.7 ## How was this patch tested It now works with IPython and Python3 Author: MechCoder <[email protected]> Closes #13503 from MechCoder/spark-15761. (cherry picked from commit 66283ee) Signed-off-by: Sean Owen <[email protected]>
…hon3 ## What changes were proposed in this pull request? I would like to use IPython with Python 3.5. It is annoying when it fails with IPython requires Python 2.7+; please install python2.7 or set PYSPARK_PYTHON when I have a version greater than 2.7 ## How was this patch tested It now works with IPython and Python3 Author: MechCoder <[email protected]> Closes apache#13503 from MechCoder/spark-15761. (cherry picked from commit 66283ee) Signed-off-by: Sean Owen <[email protected]> (cherry picked from commit 1026aba)
What changes were proposed in this pull request?
I would like to use IPython with Python 3.5. It is annoying when it fails with IPython requires Python 2.7+; please install python2.7 or set PYSPARK_PYTHON when I have a version greater than 2.7
How was this patch tested
It now works with IPython and Python3