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

Use dataclass models in YChat #119

Merged
merged 14 commits into from
Dec 16, 2024
Merged

Use dataclass models in YChat #119

merged 14 commits into from
Dec 16, 2024

Conversation

brichet
Copy link
Collaborator

@brichet brichet commented Dec 10, 2024

Fixes #115

The YChat uses dataclass models for the messages users. The User model is inherited (without change) from the jupyter server model.
It does not use dataclass for metadata, because it can contain anything. Only the id field is used by jupyterlab_chat.

This PR also save the chat file as a more readable JSON file.

EDIT: it also avoids getting all the messages in the set_message() method, where the index is already known (for performance issue on large file).

Copy link
Contributor

Binder 👈 Launch a Binder on branch brichet/jupyter-chat/dataclass

@brichet
Copy link
Collaborator Author

brichet commented Dec 10, 2024

Bumping CI

@brichet brichet closed this Dec 10, 2024
@brichet brichet reopened this Dec 10, 2024
python/jupyterlab-chat/jupyterlab_chat/ychat.py Outdated Show resolved Hide resolved
python/jupyterlab-chat/jupyterlab_chat/models.py Outdated Show resolved Hide resolved
python/jupyterlab-chat/jupyterlab_chat/ychat.py Outdated Show resolved Hide resolved
@brichet brichet marked this pull request as draft December 10, 2024 21:38
@brichet
Copy link
Collaborator Author

brichet commented Dec 10, 2024

Thanks @dlqqq for the review.
This PR needs some updates, the pycrdt.Array and pycrdt.Map do not allow dataclass.
Sorry, I should have set it as draft. It should not be related to your review, though.

@brichet brichet added the maintenance Packaging, tests label Dec 10, 2024
@brichet brichet marked this pull request as ready for review December 10, 2024 22:28
@brichet
Copy link
Collaborator Author

brichet commented Dec 10, 2024

@dlqqq In this PR I changed YChat.get_messages() to "private", because it is not returning the list of Message but a list of dict, to avoid confusion.

It will collide with jupyterlab/jupyter-ai#1151, which uses this method. I can revert this change.

@brichet brichet marked this pull request as draft December 11, 2024 08:49
@brichet brichet marked this pull request as ready for review December 12, 2024 16:00
@brichet brichet merged commit a2ddf88 into jupyterlab:main Dec 16, 2024
11 checks passed
@brichet brichet deleted the dataclass branch December 16, 2024 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Packaging, tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use dataclass models instead of untyped dictionaries in YChat Python API
3 participants