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

Don't run PYTHONSTARTUP file if a file or code is passed #5226

Merged
merged 2 commits into from Mar 6, 2014

Conversation

bouk
Copy link
Contributor

@bouk bouk commented Feb 26, 2014

This modifies Python so the behavior is how Python behaves, this is the current situation:

The same happens when you pass code to the Python binary vs. the IPython binary.

@takluyver
Copy link
Member

Actually, I think we should differ from plain Python in this situation. IPython is designed for interactive use, and a lot of the time when you run a file, you're using something like the -i flag to go into the interactive shell afterwards. Even when you're not, I think it's more relevant for us to provide a similar environment for running scripts as for interactive use. We treat PYTHONSTARTUP the same way as IPython startup files for this reason.

@airhorns
Copy link

Differing is dangerous because alias python=ipython works less and less. I use a project which makes use of PYTHONSTARTUP to launch its own interactive shell, but only in the event that a file hasn't been passed for execution on the command line. I'd like to be able to just use the same ipython binary to launch either the shell or a file from the command line and not have to make a wrapper script which makes assumptions about what command line arguments mean.

Also, this difference from normal Python is just surprising for me, I assumed it would work the same way the normal Python binary's worked based on the fact that ipython some_file.py works the same way, and the usage takes care to explain that.

That said, IPython definitely is meant for interactive use... hence the i.

@minrk
Copy link
Member

minrk commented Mar 6, 2014

We are now inclined to actually merge this. One comment in-line.

@@ -358,7 +358,7 @@ def _run_startup_files(self):
"""Run files from profile startup directory"""
startup_dir = self.profile_dir.startup_dir
startup_files = []
if self.exec_PYTHONSTARTUP:
if self.exec_PYTHONSTARTUP and not self.file_to_run and not self.code_to_run:
Copy link
Member

Choose a reason for hiding this comment

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

add self.module_to_run

@bouk
Copy link
Contributor Author

bouk commented Mar 6, 2014

I addressed the comment

@@ -358,7 +358,7 @@ def _run_startup_files(self):
"""Run files from profile startup directory"""
startup_dir = self.profile_dir.startup_dir
startup_files = []
if self.exec_PYTHONSTARTUP:
if self.exec_PYTHONSTARTUP and not self.file_to_run and not self.code_to_run and not self.module_to_run:
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick: I think this would be slightly clearer as not (self.file_to_run or self.code_to_run or self.module_to_run), which is logically equivalent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, I was wanting to do something like that

Bouke

On Thu, Mar 6, 2014 at 2:15 PM, Thomas Kluyver notifications@github.comwrote:

In IPython/core/shellapp.py:

@@ -358,7 +358,7 @@ def _run_startup_files(self):
"""Run files from profile startup directory"""
startup_dir = self.profile_dir.startup_dir
startup_files = []

  •    if self.exec_PYTHONSTARTUP:
    
  •    if self.exec_PYTHONSTARTUP and not self.file_to_run and not self.code_to_run and not self.module_to_run:
    

Nitpick: I think this would be slightly clearer as not (self.file_to_run
or self.code_to_run or self.module_to_run), which is logically equivalent.


Reply to this email directly or view it on GitHubhttps://github.com//pull/5226/files#r10355918
.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@takluyver
Copy link
Member

Great, thanks. Merging.

takluyver added a commit that referenced this pull request Mar 6, 2014
Don't run PYTHONSTARTUP file if a file or code is passed
@takluyver takluyver merged commit ce824d6 into ipython:master Mar 6, 2014
@bouk bouk deleted the fix-pythonstartup branch March 6, 2014 19:54
asfgit pushed a commit to apache/spark that referenced this pull request Apr 3, 2014
This is based on @dianacarroll's previous pull request #227, and @JoshRosen's comments on #38. Since we do want to allow passing arguments to IPython, this does the following:
* It documents that IPython can't be used with standalone jobs for now. (Later versions of IPython will deal with PYTHONSTARTUP properly and enable this, see ipython/ipython#5226, but no released version has that fix.)
* If you run `pyspark` with `IPYTHON=1`, it passes your command-line arguments to it. This way you can do stuff like `IPYTHON=1 bin/pyspark notebook`.
* The old `IPYTHON_OPTS` remains, but I've removed it from the documentation. This is in case people read an old tutorial that uses it.

This is not a perfect solution and I'd also be okay with keeping things as they are today (ignoring `$@` for IPython and using IPYTHON_OPTS), and only doing the doc change. With this change though, when IPython fixes ipython/ipython#5226, people will immediately be able to do `IPYTHON=1 bin/pyspark myscript.py` to run a standalone script and get all the benefits of running scripts in IPython (presumably better debugging and such). Without it, there will be no way to run scripts in IPython.

@JoshRosen you should probably take the final call on this.

Author: Diana Carroll <dcarroll@cloudera.com>

Closes #294 from mateiz/spark-1134 and squashes the following commits:

747bb13 [Diana Carroll] SPARK-1134 bug with ipython prevents non-interactive use with spark; only call ipython if no command line arguments were supplied

(cherry picked from commit a599e43)
Signed-off-by: Matei Zaharia <matei@databricks.com>
asfgit pushed a commit to apache/spark that referenced this pull request Apr 3, 2014
This is based on @dianacarroll's previous pull request #227, and @JoshRosen's comments on #38. Since we do want to allow passing arguments to IPython, this does the following:
* It documents that IPython can't be used with standalone jobs for now. (Later versions of IPython will deal with PYTHONSTARTUP properly and enable this, see ipython/ipython#5226, but no released version has that fix.)
* If you run `pyspark` with `IPYTHON=1`, it passes your command-line arguments to it. This way you can do stuff like `IPYTHON=1 bin/pyspark notebook`.
* The old `IPYTHON_OPTS` remains, but I've removed it from the documentation. This is in case people read an old tutorial that uses it.

This is not a perfect solution and I'd also be okay with keeping things as they are today (ignoring `$@` for IPython and using IPYTHON_OPTS), and only doing the doc change. With this change though, when IPython fixes ipython/ipython#5226, people will immediately be able to do `IPYTHON=1 bin/pyspark myscript.py` to run a standalone script and get all the benefits of running scripts in IPython (presumably better debugging and such). Without it, there will be no way to run scripts in IPython.

@JoshRosen you should probably take the final call on this.

Author: Diana Carroll <dcarroll@cloudera.com>

Closes #294 from mateiz/spark-1134 and squashes the following commits:

747bb13 [Diana Carroll] SPARK-1134 bug with ipython prevents non-interactive use with spark; only call ipython if no command line arguments were supplied
pdeyhim pushed a commit to pdeyhim/spark-1 that referenced this pull request Jun 25, 2014
This is based on @dianacarroll's previous pull request apache#227, and @JoshRosen's comments on apache#38. Since we do want to allow passing arguments to IPython, this does the following:
* It documents that IPython can't be used with standalone jobs for now. (Later versions of IPython will deal with PYTHONSTARTUP properly and enable this, see ipython/ipython#5226, but no released version has that fix.)
* If you run `pyspark` with `IPYTHON=1`, it passes your command-line arguments to it. This way you can do stuff like `IPYTHON=1 bin/pyspark notebook`.
* The old `IPYTHON_OPTS` remains, but I've removed it from the documentation. This is in case people read an old tutorial that uses it.

This is not a perfect solution and I'd also be okay with keeping things as they are today (ignoring `$@` for IPython and using IPYTHON_OPTS), and only doing the doc change. With this change though, when IPython fixes ipython/ipython#5226, people will immediately be able to do `IPYTHON=1 bin/pyspark myscript.py` to run a standalone script and get all the benefits of running scripts in IPython (presumably better debugging and such). Without it, there will be no way to run scripts in IPython.

@JoshRosen you should probably take the final call on this.

Author: Diana Carroll <dcarroll@cloudera.com>

Closes apache#294 from mateiz/spark-1134 and squashes the following commits:

747bb13 [Diana Carroll] SPARK-1134 bug with ipython prevents non-interactive use with spark; only call ipython if no command line arguments were supplied
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
Don't run PYTHONSTARTUP file if a file or code is passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants