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

Remove v2 frontend components #1156

Merged
merged 10 commits into from
Dec 20, 2024
Merged

Conversation

brichet
Copy link

@brichet brichet commented Dec 18, 2024

This PR aims to clean the v2 code.

Fixes #1146

@brichet brichet marked this pull request as draft December 18, 2024 15:51
@dlqqq dlqqq changed the title First pass to remove the front end chat Remove v2 frontend components Dec 18, 2024
@brichet brichet added maintenance Change related to maintenance of the repository @jupyter-ai/chatui labels Dec 19, 2024
@brichet
Copy link
Author

brichet commented Dec 19, 2024

Some details and questions about this PR:

@brichet brichet marked this pull request as ready for review December 19, 2024 20:31
@dlqqq
Copy link
Member

dlqqq commented Dec 19, 2024

@brichet Thanks for the info! Let me answer your questions:

Should @jupyter/chat export its rendermimeMarkdown (basically a copy of this one) and the setting panel use it ?

Sure. If @jupyter/chat provides the RendermimeMarkdown component from Jupyter AI, then it's fine to delete our copy to use yours. BTW, we should probably rename that component to something more obvious, like RenderedMarkdown.

This [chat message menu] is not yet supported in @jupyter/chat. Should we port this feature ?

No need IMO. The existing feature is a bit ugly, and the replace/insert actions are unnecessary. We will probably want some kind of chat message menu in @jupyter/chat, but this component is simple enough to be re-implemented from scratch.

Screenshot 2024-12-19 at 3 43 00 PM

Remove the generative AI menu removes the Generative AI menu from notebook cell

I think this feature should be fixed and restored

Yes, deleting this is fine. Brian and I don't see the need to include this feature as part of v3 given that it will likely be superseded by some upcoming features.

Some components are kept, for possible future usage, but can be removed:
TelemetryContext, I don't know if it's ever been used
TooltippedButton, has been ported in @jupyter/chat, so it could be exported from there if necessary
ExpandableTextField, I don't know if it's ever been used

Keep TelemetryContext as a reference, but feel free to delete the other 2.

@brichet
Copy link
Author

brichet commented Dec 20, 2024

@dlqqq thanks for your answers.

I removed the remaining components, it should be ready now.

@dlqqq dlqqq force-pushed the clean_v2_chat_code branch from c9dca76 to 011afc8 Compare December 20, 2024 19:07

test('shows chat welcome message', async () => {
await ai.assertSnapshot(FILENAMES.CHAT_WELCOME_MESSAGE);
test('Should be tested', () => {
Copy link
Member

Choose a reason for hiding this comment

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

lmao

Copy link
Member

@dlqqq dlqqq left a comment

Choose a reason for hiding this comment

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

Tested it and this works well, thank you so much for helping with this! 🤗

There's probably still a few unused JS files, but we can iterate on this in future PRs.

@dlqqq
Copy link
Member

dlqqq commented Dec 20, 2024

Note that the Python 3.9 tests are failing because of the kw_only argument being passed to the dataclass decorators, new to jupyterlab-chat:

@jupyter-ai/core:   warnings.warn("An error occurred.")
@jupyter-ai/core: /opt/hostedtoolcache/Python/3.9.20/x64/lib/python3.9/site-packages/jupyterlab/debuglog.py:55: UserWarning: ModuleNotFoundError: There is no labextension at .. Errors encountered: [TypeError("the 'package' argument is required to perform a relative import for '.'"), TypeError("dataclass() got an unexpected keyword argument 'kw_only'"), TypeError("dataclass() got an unexpected keyword argument 'kw_only'"), TypeError("dataclass() got an unexpected keyword argument 'kw_only'"), TypeError("dataclass() got an unexpected keyword argument 'kw_only'"), TypeError("dataclass() got an unexpected keyword argument 'kw_only'"), TypeError("dataclass() got an unexpected keyword argument 'kw_only'"), TypeError("dataclass() got an unexpected keyword argument 'kw_only'"), TypeError("dataclass() got an unexpected keyword argument 'kw_only'"), TypeError("dataclass() got an unexpected keyword argument 'kw_only'"), TypeError("dataclass() got an unexpected keyword argument 'kw_only'"), TypeError("dataclass() got an unexpected keyword argument 'kw_only'"), TypeError("dataclass() got an unexpected keyword argument 'kw_only'"), TypeError("dataclass() got an unexpected keyword argument 'kw_only'"), TypeError("dataclass() got an unexpected keyword argument 'kw_only'"), TypeError("dataclass() got an unexpected keyword argument 'kw_only'"), TypeError("dataclass() got an unexpected keyword argument 'kw_only'"), TypeError("dataclass() got an unexpected keyword argument 'kw_only'"), TypeError("dataclass() got an unexpected keyword argument 'kw_only'"), TypeError("dataclass() got an unexpected keyword argument 'kw_only'"), TypeError("dataclass() got an unexpected keyword argument 'kw_only'"), TypeError("dataclass() got an unexpected keyword argument 'kw_only'"), TypeError("dataclass() got an unexpected keyword argument 'kw_only'"), TypeError("dataclass() got an unexpected keyword argument 'kw_only'"), TypeError("dataclass() got an unexpected keyword argument 'kw_only'"), TypeError("dataclass() got an unexpected keyword argument 'kw_only'"), TypeError("dataclass() got an unexpected keyword argument 'kw_only'"), TypeError("dataclass() got an unexpected keyword argument 'kw_only'"), TypeError("dataclass() got an unexpected keyword argument 'kw_only'")]

kw_only is only available in Python 3.10+: https://docs.python.org/3/library/dataclasses.html#dataclasses.dataclass

@dlqqq
Copy link
Member

dlqqq commented Dec 20, 2024

Opened a new issue upstream to track this: jupyterlab/jupyter-chat#134

@dlqqq dlqqq merged commit b5441ae into jupyterlab:v3-dev Dec 20, 2024
8 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@jupyter-ai/chatui maintenance Change related to maintenance of the repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants