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

Support stdin output #233

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions jupyter_ydoc/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from .yblob import YBlob as YBlob
from .yfile import YFile as YFile
from .ynotebook import YNotebook as YNotebook
from .ystdin import add_stdin as add_stdin
from .yunicode import YUnicode as YUnicode

# See compatibility note on `group` keyword in
Expand Down
4 changes: 4 additions & 0 deletions jupyter_ydoc/ynotebook.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,10 @@ def get_cell(self, index: int) -> Dict[str, Any]:
and not cell["attachments"]
):
del cell["attachments"]
outputs = cell.get("outputs", [])
for idx, output in enumerate(outputs):
if output["output_type"] == "stdin":
del outputs[idx]
return cell

def append_cell(self, value: Dict[str, Any]) -> None:
Expand Down
36 changes: 36 additions & 0 deletions jupyter_ydoc/ystdin.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# Copyright (c) Jupyter Development Team.
# Distributed under the terms of the Modified BSD License.

from pycrdt import Map, Text


def add_stdin(cell: Map, prompt: str = "", password: bool = False) -> None:
"""
Adds an stdin Map in the cell outputs.

Schema:

.. code-block:: json

{
"state": Map[
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a question we do not need to answer now: should there be an owner of the input box too? So that the input can be provided only by the user who executed the code.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If we think inputs are collaborative, then anybody should be able to fill them, right?
Otherwise I don't think this is really compatible with CRDTs, that are essentially shared structures.

Copy link
Collaborator

Choose a reason for hiding this comment

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

A teacher may want students to see what they are typing, but not allow students to type in into their box.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

At first sight it looks like the same issue as with cell code: there is no way to prevent a student from changing the code. Why would it be different with a input? If it's because it's the teacher who executed the cell, well this information is not present in the shared model, and I don't think it should be. And anyway, a student can re-execute the cell, and now should they be able to type in into the box? It doesn't really make sense.
A clearer model would be that the whole notebook is read-only for students. In this case we could ignore their Y-updates so that they can't change the shared model. This would make use of user permissions.
This is a general constraint of CRDTs, everybody sees the same data, unlike in a server/client architecture where a client makes a request and receives a dedicated reply. For instance, paginated results are possible with HTTP but not with CRDTs.

"pending": bool,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think there are no non-pending inputs, they get cleared away once the result was submitted.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Though maybe there will be fore example if the kernel dies. Should we then clear the input box or keep it with some kind of disconnected icon?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

By "pending" I meant that the input is in a "being edited" state or a "submitted" state, in which case it cannot be edited again.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Though maybe there will be fore example if the kernel dies. Should we then clear the input box or keep it with some kind of disconnected icon?

In this case I guess we should just remove the stdin output from the shared model, because the kernel is not actually waiting for an input anymore, right?

"password": bool
],
"prompt": str,
"input": Text
}
"""
stdin = Map(
{
"output_type": "stdin",
"state": {
"pending": True,
"password": password,
},
"prompt": prompt,
"input": Text(),
}
)
outputs = cell.get("outputs")
outputs.append(stdin)
33 changes: 32 additions & 1 deletion tests/test_ydocs.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Copyright (c) Jupyter Development Team.
# Distributed under the terms of the Modified BSD License.

from jupyter_ydoc import YBlob
from jupyter_ydoc import YBlob, YNotebook, add_stdin


def test_yblob():
Expand All @@ -22,3 +22,34 @@ def callback(topic, event):
assert topic == "source"
assert event.keys["bytes"]["oldValue"] == b"012"
assert event.keys["bytes"]["newValue"] == b"345"


def test_stdin():
ynotebook = YNotebook()
ynotebook.append_cell(
{
"cell_type": "code",
"source": "",
}
)
ycell = ynotebook.ycells[0]
add_stdin(ycell)
cell = ycell.to_py()
# cell ID is random, ignore that
del cell["id"]
assert cell == {
"outputs": [
{
"output_type": "stdin",
"input": "",
"prompt": "",
"state": {
"password": False,
"pending": True,
},
}
],
"source": "",
"metadata": {},
"cell_type": "code",
}
Loading