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

Switch from 'embedding' to 'starting' IPython #507

Merged
merged 2 commits into from Oct 29, 2020

Conversation

kemitche
Copy link
Contributor

@kemitche kemitche commented Oct 28, 2020

Embedding IPython results in issues where import statements and other
globals changes in the shell environment don't work as expected.[1]
As an example, the following commands result in a NameError:

In [1]: import time

In [2]: def foo():
   ...:     print(time.time())
   ...:

In [3]: foo()
---------------------------------------------------------------------------
NameError                                 Traceback (most recent call last)
<ipython-input-3-c19b6d9633cf> in <module>
----> 1 foo()

<ipython-input-2-a038e7f91c93> in foo()
      1 def foo():
----> 2     print(time.time())
      3

NameError: name 'time' is not defined

By switching to use "start_ipython()" instead, we get an environment
more similar to what we get by running ipython directly.

In [1]: import time

In [2]: def foo():
   ...:     print(time.time())
   ...: 

In [3]: foo()
1603919279.0674398

[1] ipython/ipython#62

Copy link
Contributor

@spladug spladug left a comment

Choose a reason for hiding this comment

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

Sweet! Thanks for doing this. I think this just needs the banner and then is good to go.

from IPython.terminal.embed import InteractiveShellEmbed
from IPython.core.interactiveshell import DummyMod

shell = InteractiveShellEmbed(banner2=banner)
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like the banner isn't getting shown.

Copy link
Contributor

Choose a reason for hiding this comment

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

ubuntu@reddit:~/src/harold2 (harold2)$ onevm shell
Running “baseplate-shell /home/ubuntu/src/harold2/example.ini”…
INFO:raven.base.Client:Raven is not configured (logging is disabled). Please see the documentation for more information.
Python 3.8.5 (default, Jul 20 2020, 19:48:14) 
Type 'copyright', 'credits' or 'license' for more information
IPython 7.18.1 -- An enhanced Interactive Python. Type '?' for help.

In [1]: context
Out[1]: <baseplate.RequestContext at 0x7fdd587c3610>

so the values are there as expected but there's no message explaining how to use 'em.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, let me get the banner back into place! (I'm so used to it at this point I've gloss right over it)

Embedding IPython results in issues where import statements and other
globals changes in the shell environment don't work as expected.[1]
As an example, the following commands result in a NameError:

In [1]: import time

In [2]: def foo():
   ...:     print(time.time())
   ...:

In [3]: foo()
---------------------------------------------------------------------------
NameError                                 Traceback (most recent call last)
<ipython-input-3-c19b6d9633cf> in <module>
----> 1 foo()

<ipython-input-2-a038e7f91c93> in foo()
      1 def foo():
----> 2     print(time.time())
      3

NameError: name 'time' is not defined

By switching to use "start_ipython()" instead, we get an environment
more similar to what we get by running `ipython` directly.

[1] ipython/ipython#62
Copy link
Contributor

@spladug spladug left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks so much!

@kemitche kemitche merged commit 705ff03 into reddit:develop Oct 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants