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

Make json_clean a no-op for jupyter-client >= 7 #708

Merged
merged 1 commit into from Sep 7, 2021

Conversation

martinRenou
Copy link
Contributor

Fix #706

This should not be merged before a potential release of jupyter_client with jupyter/jupyter_client#664, otherwise, Matplotlib will not work anymore in Jupyter Notebook/Lab

@JohanMabille
Copy link
Contributor

JohanMabille commented Jul 4, 2021

Please notice that this function is also used by the debugger and this PR would break the richInspect feature. It should be fixed before merging this.

@martinRenou
Copy link
Contributor Author

Thanks for the review @JohanMabille. I hadn't rebased.

@martinRenou
Copy link
Contributor Author

This should be resolved now. Testing message dumps as jupyter_client does it.

@minrk
Copy link
Member

minrk commented Jul 6, 2021

👍 to the change in general. Can we get matplotlib working? Or can it not be done here because the mpl backend was moved to its own package?

What about including a backport of the json_default function from jupyter/jupyter_client#664?

@martinRenou
Copy link
Contributor Author

Thanks a lot for your review @minrk !

Can we get matplotlib working?

This needs jupyter/jupyter_client#664 which does the base64 serializing https://github.com/jupyter/jupyter_client/pull/664/files#diff-fda072ac1fb480ce6ce75694ef6e4144e811d8443191be1c4f233a9ee3fd02a1R108

I am not sure we can fix it here, as ipykernel is not responsible for the serialization.

What about including a backport of the json_default function from jupyter/jupyter_client#664?

You mean here in ipykernel?

@martinRenou martinRenou force-pushed the clean_json_clean branch 2 times, most recently from a4ea628 to 61be612 Compare August 16, 2021 15:16
@martinRenou
Copy link
Contributor Author

martinRenou commented Aug 16, 2021

This requires jupyter_client>=7, I've updated the requirement in setup.py accordingly

I made the json_clean function a no-op only when jupyter_client>=7

@martinRenou martinRenou changed the title Remove json_clean Make json_clean a no-op for jupyter-client >= 7 Sep 6, 2021
@martinRenou martinRenou force-pushed the clean_json_clean branch 3 times, most recently from a3cea30 to 9784c61 Compare September 6, 2021 12:28
@martinRenou martinRenou marked this pull request as ready for review September 7, 2021 08:45
@blink1073 blink1073 added this to the 6.3 milestone Sep 7, 2021
Copy link
Member

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

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

Thank you!

@blink1073 blink1073 merged commit 3cee9ce into ipython:master Sep 7, 2021
@martinRenou martinRenou deleted the clean_json_clean branch September 7, 2021 09:38
@martinRenou
Copy link
Contributor Author

Thanks!!

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.

Remove json_clean
4 participants