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

Add the "Open with Classic Notebook" entry in the interface dropdown if NBClassic is available? #6746

Closed
jtpio opened this issue Feb 23, 2023 · 11 comments · Fixed by #6866
Closed
Assignees
Milestone

Comments

@jtpio
Copy link
Member

jtpio commented Feb 23, 2023

Currently the latest Notebook 7 pre-release only lists JupyterLab as an alternative interface for opening the current notebook:

image

When it was added in RetroLab it also displayed the classic notebook because notebook was installed automatically as a transitive dependency: jupyterlab/retrolab#309

interface-switch-menu.mp4

Currently nothing installs nbclassic by default anymore so the "Open with Classic Notebook" entry is not displayed.

But maybe the interface switcher plugin should try to detect if nbclassic is available, and add an extra menu entry that points to /nbclassic/notebooks if it is?

@jtpio jtpio added this to the 7.0 milestone Feb 23, 2023
@jupyterlab-probot jupyterlab-probot bot added the status:Needs Triage Applied to issues that need triage label Feb 23, 2023
@jtpio jtpio added the question label Feb 23, 2023
@jtpio
Copy link
Member Author

jtpio commented Feb 23, 2023

The relevant part of the code is located here:

if (!notebookShell) {
addInterface({
command: CommandIDs.openNotebook,
commandLabel: trans.__('Open With %1', 'Jupyter Notebook'),
buttonLabel: 'openNotebook',
urlPrefix: `${baseUrl}tree/`
});
}
if (!labShell) {
addInterface({
command: CommandIDs.openLab,
commandLabel: trans.__('Open With %1', 'JupyterLab'),
buttonLabel: 'openLab',
urlPrefix: `${baseUrl}doc/tree/`
});
}

@yuvipanda
Copy link
Contributor

I would really like this to be part of the release! Adding that to retrolab was how we were able to convince many instructors to give retrolab a try, and I think it's a low effort high impact thing - being able to say 'you can switch to the older interface at any time' dissolves a lot of resistance, even if that is unused.

@jtpio
Copy link
Member Author

jtpio commented Apr 3, 2023

Reposting #6793 (comment) here for visibility:

  1. either we add a new entry in the dropdown pointing to /nbclassic/notebook, but without depending on nbclassic in notebook. The link might give a 404 if nbclassic is not installed. But would work if nbclassic is installed. Users will have to explicitly pip install nbclassic to get the classic notebook UI
  2. or we add the new entry as mentioned in 1., and also make notebook==7 depend on nbclassic to make sure it's always there.

@yuvipanda suggests to go with 2 to make the Notebook 7 rollout smoother.

cc @jupyter/notebook-council for awareness and thoughts about this

@echarles
Copy link
Member

echarles commented Apr 3, 2023

  1. either we add a new entry in the dropdown pointing to /nbclassic/notebook, but without depending on nbclassic in notebook. The link might give a 404 if nbclassic is not installed. But would work if nbclassic is installed. Users will have to explicitly pip install nbclassic to get the classic notebook UI

This does not sound like a good option to me.

  1. or we add the new entry as mentioned in 1., and also make notebook==7 depend on nbclassic to make sure it's always there.

That will work. I don't see direct negative impact as we have designed the system to support that case. For sure there are not a lot of such env in the field, meaning that is not widely tested and issues might not have been reported yet.

  1. Another option would be to detect on server launch if nbclassic is present or not and inject that information in the pageconfig, so that Notebook 7 adapts its ui/menu accordingly. That is some extra development work. BTW this could be generalised to detect the existence of some well-targeted packages and that feature could be useful to other, so would need maybe to be implemented in JupyterLab, probably too late today due to the current beta/rc release cycle for JupyterLab 4.

@yuvipanda
Copy link
Contributor

I also like (2), extended to (3) in the future if resources exist to do so.

@ellisonbg
Copy link
Contributor

I think it is a mistake to have a hard dependency on nbclassic in notebook 7. For many users, they won't know the difference and it will confuse them (why do I have the current and last release of an application at the same time when they look and act nearly identical). This also makes it much more difficult for operators that don't want to deploy nbclassic.

@ellisonbg
Copy link
Contributor

I think that option (3) above (detect nbclassic dynamically) would be fine. If this is the right thing for users, we can help get this done for the notebook 7 release.

@echarles
Copy link
Member

echarles commented Apr 5, 2023

This also makes it much more difficult for operators that don't want to deploy nbclassic.

Makes sense.

I think that option (3) above (detect nbclassic dynamically) would be fine. If this is the right thing for users, we can help get this done for the notebook 7 release.

Super. We have our developer meeting today and @andrii-i typically joins. If Andrii is entitled to take that action point, it will be great to discuss about that today.

@ellisonbg
Copy link
Contributor

Great, Andrii would be the one who can do that work, go ahead and mention it to him.

@andrii-i
Copy link
Contributor

andrii-i commented Apr 5, 2023

@echarles @ellisonbg was missing the the context before the Notebook call; would be happy to work on this, please assign this issue to me

@jtpio
Copy link
Member Author

jtpio commented Apr 27, 2023

An alternative to making notebook depend on nbclassic would be to add nbclassic to the jupyter metapackage instead: https://github.com/jupyter/jupyter

The jupyter metapackage is very popular. Users often "pip install jupyter" when they are told to "Install Jupyter".

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.