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

Stop using IPython.lib.kernel 0.13.2 shim and add initial support for Jupyter #2486

Merged
merged 2 commits into from
Jun 16, 2015

Conversation

SylvainCorlay
Copy link
Member

  • find_connection_file was being imported from IPython.lib.kernel which is a shim to IPython.kernel.connect since 1.0. Therefore, this does not change anything for IPython 1.0 to 3.2. We are just using a more direct path.
  • Starting with IPython 4.0,
    • IPython.lib.kernel is a shim to ipykernel.connect
    • IPython.kernel.connect is a shim as well, which first imports everything from ipykernel.connect and then everything from jupyter_client.connect. The latter sets the default directory for connection files from ~/.ipython/profile_*/security to JUPYTER_DATA_DIR, which is the correct behavior.

@SylvainCorlay
Copy link
Member Author

OK, this solves the last issue I had with Jupyter, where find_connection_file has a slightly different signature.

@minrk
Copy link
Contributor

minrk commented Jun 13, 2015

Minor correction: jupyter connection files go in JUPYTER_DATA_DIR, not ~/.jupyter, and Jupyter has no profiles, so there is no ~/.jupyter/profile_*.

@SylvainCorlay
Copy link
Member Author

Thanks @minrk I edited the description.

@stonebig
Copy link
Contributor

could it ship in coming Spyder 2.3.5 ?

@SylvainCorlay
Copy link
Member Author

@stonebig good point, I guess we should do it if 3.0 does not ship before IPython 4.0.

@ccordoba12
Copy link
Member

So with this PR we will be compatible with IPython/Jupyter 4.0?

@SylvainCorlay
Copy link
Member Author

Yes, it works with the current development version.

@ccordoba12 ccordoba12 changed the title Stop using 0.13.2 shim Stop using IPython.lib.kernel 0.13.2 shim Jun 15, 2015
@ccordoba12
Copy link
Member

Ok, thanks @SylvainCorlay for the confirmation.

@stonebig, I'll backport these changes to release them in 2.3.5.

@stonebig
Copy link
Contributor

Thanks.

@SylvainCorlay
Copy link
Member Author

  • IPython.config.loader is shimed to traitlets.config.loader, so the function load_pyconfig_files is found. In the case it does not find connection files, nothing happens. Hence 2 and 3 should not create any issue. I guess we want to keep this though for backward compatibility.
  • Regarding 1, I wonder whether we should clean up for the connection file at all.
  • Remark: We could add comments wherever we use IPython shims, so that we know where to import from when we drop support for older versions.

@ccordoba12
Copy link
Member

I'm going to merge this to have some kind of support for Jupyter 4.0. Later we can see what we can do about 3.

@SylvainCorlay, please open a new PR with the comments you suggest. I guess there are going to be several of them :-)

@ccordoba12 ccordoba12 changed the title Stop using IPython.lib.kernel 0.13.2 shim Stop using IPython.lib.kernel 0.13.2 shim and initial support for Jupyter Jun 16, 2015
@ccordoba12 ccordoba12 changed the title Stop using IPython.lib.kernel 0.13.2 shim and initial support for Jupyter Stop using IPython.lib.kernel 0.13.2 shim and add initial support for Jupyter Jun 16, 2015
ccordoba12 added a commit that referenced this pull request Jun 16, 2015
Stop using IPython.lib.kernel 0.13.2 shim and add initial support for Jupyter
@ccordoba12 ccordoba12 merged commit 6e4bc8f into spyder-ide:master Jun 16, 2015
@SylvainCorlay
Copy link
Member Author

Thanks. I am looking into the points you raised and it is not immediate. I should come up with another PR soon.

@ccordoba12
Copy link
Member

Ok, thanks!

@SylvainCorlay SylvainCorlay deleted the old_shim branch June 16, 2015 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants