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

PR: Trade ipykernel.serialize for cloudpickle and remove use of publish_data #5341

Merged
merged 16 commits into from
Nov 30, 2017

Conversation

minrk
Copy link
Contributor

@minrk minrk commented Sep 27, 2017

  • use custom spyder_msg message type rather than IPython Parallel-specific data_pub.
    This removes the need for special __spy__ names because it is no longer a shared namespace.
  • use cloudpickle for serializing objects rather than deprecated ipykernel.serialize
    that is reserved for IPython Parallel.

@pep8speaks
Copy link

pep8speaks commented Sep 27, 2017

Hello @minrk! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on November 29, 2017 at 17:51 Hours UTC

@ccordoba12
Copy link
Member

ccordoba12 commented Sep 27, 2017

@minrk, thanks a lot for taking the time to help us improve our architecture! You know better than anyone else the Jupyter/IPython intricacies, so we really appreciate it.

We were thinking to use dill instead of cloudpickle to improve serialization, because dill has also the ability to restore a full IPython interpreter session, something we'd like to add in the future.

Any particular advice for using cloudpickle instead?

@ccordoba12 ccordoba12 changed the title trade deprecated data_pub for cloudpickle PR: Trade deprecated data_pub for cloudpickle Sep 27, 2017
@minrk
Copy link
Contributor Author

minrk commented Sep 27, 2017

No, I don't think there's a big separator between dill and cloudpickle. cloudpickle's just my default choice these days. No objection to switching to dill.

minrk and others added 5 commits November 26, 2017 23:14
for easier dispatching without sharing a namespace with user messages
that's 5 files now for every dependency - setup.py, requirements.txt, install.sh, one for each CI
@ccordoba12 ccordoba12 added this to the v3.2.5 milestone Nov 27, 2017
@ccordoba12 ccordoba12 changed the base branch from master to 3.x November 27, 2017 05:59
@ccordoba12 ccordoba12 closed this Nov 27, 2017
@ccordoba12 ccordoba12 reopened this Nov 27, 2017
@ccordoba12 ccordoba12 changed the title PR: Trade deprecated data_pub for cloudpickle PR: Trade deprecated datapub for cloudpickle Nov 27, 2017
@ccordoba12 ccordoba12 changed the title PR: Trade deprecated datapub for cloudpickle PR: Trade publish_data for cloudpickle Nov 27, 2017
@ccordoba12 ccordoba12 changed the title PR: Trade publish_data for cloudpickle PR: Trade ipykernel.serialize for cloudpickle and remove use of publish_data Nov 29, 2017
@ccordoba12
Copy link
Member

Thanks a lot @minrk for this! It's a really great contribution!!

I decided it to move it to our stable branch to avoid Spyder to fail when ipykernel 5.0 is released. I also fixed several issues with Python 2.

@ccordoba12 ccordoba12 merged commit c65e94e into spyder-ide:3.x Nov 30, 2017
ccordoba12 added a commit that referenced this pull request Nov 30, 2017
@ccordoba12
Copy link
Member

Pinging @ghisvail about this one because it adds cloudpickle as a new dep. We hope to not add more new deps in 3.x, but this one was really necessary.

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.

3 participants