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

Jupyter Debugger Protocol #47

Merged
merged 4 commits into from
Jul 27, 2020

Conversation

SylvainCorlay
Copy link
Member

@SylvainCorlay SylvainCorlay commented Jan 1, 2020

@bollwyvl
Copy link

bollwyvl commented Jan 1, 2020

Great work, all, on the stuff leading up to this!

+1 to DAP: it has a well-defined JSON schema, and the specification is generated from it.

Indeed, when integrating this into the documentation, it may be appropriate to formalize the Jupyter Kernel Messaging spec as a whole into a (set of) JSON schema vs the status quo of some .rst and a reference implementation, which similarly plagues LSP(#26). This would also be helpful for updating the aging jupyter/jupyter_kernel_test.

@SylvainCorlay
Copy link
Member Author

Indeed, when integrating this into the documentation, it may be appropriate to formalize the Jupyter Kernel Messaging spec as a whole into a (set of) JSON schema vs the status quo of some .rst and a reference implementation, which similarly plagues LSP(#26).

Absolutely!

@afshin
Copy link
Member

afshin commented Jan 2, 2020

👍

I think this will open the way for many other kernel authors to opt into offering a debugging experience for users.

@SylvainCorlay
Copy link
Member Author

Thanks for chiming in @captainsafia! I don't know which level of detail should go in the JEP vs the individual PRs but I wanted to get the ball rolling now that we have an implementation.

@meeseeksmachine
Copy link

This pull request has been mentioned on Jupyter Community Forum. There might be relevant details there:

https://discourse.jupyter.org/t/sustainability-of-the-ipynb-nbformat-document-format/3051/2

Copy link
Member

@jasongrout jasongrout left a comment

Choose a reason for hiding this comment

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

Thanks!

@SylvainCorlay
Copy link
Member Author

ping @takluyver @Carreau @minrk @ivanov

I would love to have your vote on this since you worked so much on the original protocol.

@Carreau
Copy link
Member

Carreau commented Feb 4, 2020

Approving.

Copy link
Member

@fperez fperez left a comment

Choose a reason for hiding this comment

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

This is great and I'm very happy to see the support.

One question that came to mind and that I think for completeness should be recorded in the JEP, as others may be equally curious - what the connection of this work is with %debug and ipdb.

I suspect the answer is that ipdb, being much older than these standards, is its own little private world unaffected by this new model. Interaction with ipdb happens strictly over stdin/out (forwarded over the right messaging ports, which was an important part of completing the messaging support early on, with @minrk doing a lot of that). But still, it should be documented as %debug has been our in-house workhorse for a long time, and will remain a valid tool for users.

@jasongrout
Copy link
Member

jasongrout commented Jul 21, 2020

This proposal has been open over 6 months, and has now earned 70% steering council approval plus several approvals from the wider community. @SylvainCorlay - do you want to address Fernando's comment about adding a remark about %debug? Then I'll merge this as approved (unless there are further objections raised).

@SylvainCorlay
Copy link
Member Author

Do you want to address Fernando's comment about adding a remark about %debug

I just added a remark about the %debug magic in the introduction.

@Carreau
Copy link
Member

Carreau commented Jul 27, 2020

Jason said:

Then I'll merge this as approved (unless there are further objections raised).

As he said it 6 day ago, I'm going to merge. Failure is unrelated.

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.