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

Jupyterlab extensions #1588

Merged
merged 139 commits into from
Jun 16, 2022
Merged

Jupyterlab extensions #1588

merged 139 commits into from
Jun 16, 2022

Conversation

brichet
Copy link
Contributor

@brichet brichet commented Jun 1, 2022

This PR brings the jupyterlab extensions to nbgrader.

It is an improvement of the work done by @Lawrence37 and @aalmanza1998 in https://github.com/LibreTexts/nbgrader-to-jupyterlab/tree/lab-common.

It works with jupyterlab < 4 and is still compatible with classic notebook (nbextensions still work).

Still missing some tests on labextensions (formgrader and course list), which could be added in a future PR.

@fcollonval also helped with this PR.

Fix #1006
Related to #1335

@jtpio jtpio added this to the 0.8.0 milestone Jun 1, 2022
@jhamrick
Copy link
Member

jhamrick commented Jun 2, 2022

Awesome---thanks a lot for this! Will try to have a look sometime next week...

@brichet
Copy link
Contributor Author

brichet commented Jun 2, 2022

Working on documentation I just realize that there are some problems with images being duplicated.
This could be related to nbconvert as the images in error are from the Notebook files in the documentation ("Creating and grading assignments" and "Exchanging assignment files" in TOC).

@jtpio
Copy link
Member

jtpio commented Jun 2, 2022

Nice!

Probably this could in some way (or partially) also be tested on Binder with the following link: https://mybinder.org/v2/gh/brichet/nbgrader/jupyterlab_extensions?urlpath=lab

@perllaghu
Copy link
Contributor

Nice!

Probably this could in some way (or partially) also be tested on Binder with the following link: https://mybinder.org/v2/gh/brichet/nbgrader/jupyterlab_extensions?urlpath=lab

Poodles.... I'm getting a No space left on device error from Binder :(

@perllaghu
Copy link
Contributor

I'm looking at this in a notebook, How does one access Formgrader & Assignment list sections?

If I switch to the classic interface, they're there....

@brichet
Copy link
Contributor Author

brichet commented Jun 3, 2022

I'm looking at this in a notebook, How does one access Formgrader & Assignment list sections?

If I switch to the classic interface, they're there....

I will update the documentation soon.
Extensions are available from command palette (Ctrl + Shift + c) and then you can type formgrader or assignment list, it will open a new tab in Jupyter Lab.

@brichet
Copy link
Contributor Author

brichet commented Jun 3, 2022

Working on documentation I just realize that there are some problems with images being duplicated. This could be related to nbconvert as the images in error are from the Notebook files in the documentation ("Creating and grading assignments" and "Exchanging assignment files" in TOC).

For traceability :
According to spatialaudio/nbsphinx#162 it seems that using pandoc>=2 changes the way images are linked into RST file (from Markdown). This was causing errors when using nbconvert to convert some documentation Notebook to RST (WARNING: Duplicate substitution definition name: "image0").
Whenever a markdown cell containing images was converted, the first image was substituted by "image0" definition name, causing the duplication error if more than one cell contained images (it was the same for "image1" and so on).
I assume that using readthedoc.yaml v2 brought that behavior.
This is simply fixed by adding alternative text for the images in the markdown (spatialaudio/nbsphinx#162 (comment)).

@perllaghu
Copy link
Contributor

Everything's looking good..... I've now got the full assignment cycle running - even with my plugin exchange service!

The only thing not working now (as far as I can see) is the "View Feedback" link. The feedback is downloaded, and the link is added to the submitted item, however the link is failing.

Works fine in the classic interface, where it has [for me] an href of ...tree/<course>/<assignment>/feedback/<timestamp>

In the lab UI, the <a> element is completely attribute-less: <a> (view feedback)</a>

@brichet
Copy link
Contributor Author

brichet commented Jun 3, 2022

Everything's looking good..... I've now got the full assignment cycle running - even with my plugin exchange service!

The only thing not working now (as far as I can see) is the "View Feedback" link. The feedback is downloaded, and the link is added to the submitted item, however the link is failing.

Works fine in the classic interface, where it has [for me] an href of ...tree/<course>/<assignment>/feedback/<timestamp>

In the lab UI, the <a> element is completely attribute-less: <a> (view feedback)</a>

Thanks @perllaghu for your feedback.
From my side the view feedback is working, but maybe the feature could be handled better. As there can be several feedbacks, nbextension opens a tree file tab.
In this extension, instead of opening a new tab, it simply opens the file browser (left panel) in the correct directory.
If you change the directory in the file browser, the link should open ...tree/<course>/<assignment>/feedback/<timestamp> in it.

@perllaghu
Copy link
Contributor

perllaghu commented Jun 4, 2022

Thanks @perllaghu for your feedback. From my side the view feedback is working, but maybe the feature could be handled better. As there can be several feedbacks, nbextension opens a tree file tab. In this extension, instead of opening a new tab, it simply opens the file browser (left panel) in the correct directory. If you change the directory in the file browser, the link should open ...tree/<course>/<assignment>/feedback/<timestamp> in it.

OK..... that's not happening in the Docker Images I have running in the K8 cluster.

Let me try in a basic image - there may be something in my alternative exchange code that's throwing it..

edit...
Yep, works in a basic Docker image.

@brichet
Copy link
Contributor Author

brichet commented Jun 6, 2022

Thanks @perllaghu for your feedback. From my side the view feedback is working, but maybe the feature could be handled better. As there can be several feedbacks, nbextension opens a tree file tab. In this extension, instead of opening a new tab, it simply opens the file browser (left panel) in the correct directory. If you change the directory in the file browser, the link should open ...tree/<course>/<assignment>/feedback/<timestamp> in it.

OK..... that's not happening in the Docker Images I have running in the K8 cluster.

Let me try in a basic image - there may be something in my alternative exchange code that's throwing it..

edit... Yep, works in a basic Docker image.

Do you think this is something to fix ? How can we reproduce it ?

@perllaghu
Copy link
Contributor

OK..... that's not happening in the Docker Images I have running in the K8 cluster.
Let me try in a basic image - there may be something in my alternative exchange code that's throwing it..
edit... Yep, works in a basic Docker image.

Do you think this is something to fix ? How can we reproduce it ?

I've not got any further over the weekend.... however I think it's more likely to be a local problem that a core one - I'll have some pythonic library installed that's conflicting.

@perllaghu
Copy link
Contributor

In a separate thought-stream: did you consider adding the formgrader and/or assignment list commands to the Jupyterlab Interface?

I think these are correct - it's part of the typescript in src/index.ts:

  // Add the command to the launcher
  if (launcher) {
    launcher.add({
      command,
      category: 'Other',
      rank: 1
    });
  }

and/or

  // Add the command to the menu
  if (menu) {
    menu.fileMenu.newMenu.addGroup([{ command }], 30);
  }

(I was playing with something different over at https://github.com/edina/noteable-htmlProject )

@perllaghu
Copy link
Contributor

OK..... that's not happening in the Docker Images I have running in the K8 cluster.
Let me try in a basic image - there may be something in my alternative exchange code that's throwing it..
edit... Yep, works in a basic Docker image.

Do you think this is something to fix ? How can we reproduce it ?

I've not got any further over the weekend.... however I think it's more likely to be a local problem that a core one - I'll have some pythonic library installed that's conflicting.

Basic image works in the cluster.... so there's something added to my default image that breaks it.... and that [mostly] makes it my problem :)

@brichet
Copy link
Contributor Author

brichet commented Jun 6, 2022

In a separate thought-stream: did you consider adding the formgrader and/or assignment list commands to the Jupyterlab Interface?

I added menu entries for formgrader, assignment list and course list.

Copy link
Member

@jhamrick jhamrick 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 looking really nice, thanks for the work on it! I found a few issues in my testing, described below. I tested in a clean mamba env using a fresh course directory created by nbgrader quickstart. I didn't really look at the code much, just a skim through.

  • In the assignment list, the formatting causes the status messages to be clipped on the left side. This is partially aesthetic, but if you get the error that the exchange is not setup correctly, it hides the "h" from "http" in the the provided documentation url which will definitely confuse some people. Can the formatting be adjusted to fix this so there's no clipping?

image

  • The "Create Assignment" extension doesn't seem to work for me. It appears on the right hand side as in the screenshots, but when I click on it nothing happens.

  • When I run "Validate" on the instructor notebook through lab after having tried to activate the "Create Assignment" extension, I get the following error: "The nbgrader ID "null" has been used for more than one cell. Please make sure all grade cells have unique ids." This does not occur when running validate from the traditional notebook interface. It also does not occur if I haven't tried to use the "Create Assignment" extension.

image

  • Mathjax does not seem to render when viewing generated feedback in a tab in lab, but it does render when viewing the same file directly in the browser. Example in lab:

image

Same file in browser:

image

  • Validation output looks different depending on whether you run it from the assignment list extension or in the notebook itself (and both of these look different from how it appears when using the classic notebook, which shows identical output regardless of whether it's generated from the assignment list extension or from the notebook). When running validation from the notebook in lab, the errors are displayed with a pink background and are not scrollable:

image

When running validation from the assignment list extension in lab, the errors do not have a pink background and are displayed in scrollable boxes:

image

Classic notebook:

image

  • When building the docs, I get a bunch of warnings that should probably be addressed:
/Users/jhamrick/project/nbgrader/nbgrader/docs/source/configuration/jupyterhub_config.rst:2: WARNING: Duplicate explicit target name: "jupyterhub documentation".
/Users/jhamrick/project/nbgrader/nbgrader/docs/source/user_guide/creating_and_grading_assignments_nb.rst:124: WARNING: duplicate label manually-graded-cells, other instance in /Users/jhamrick/project/nbgrader/nbgrader/docs/source/user_guide/creating_and_grading_assignments_jlab.rst
/Users/jhamrick/project/nbgrader/nbgrader/docs/source/user_guide/creating_and_grading_assignments_nb.rst:148: WARNING: duplicate label manually-graded-task-cells, other instance in /Users/jhamrick/project/nbgrader/nbgrader/docs/source/user_guide/creating_and_grading_assignments_jlab.rst
/Users/jhamrick/project/nbgrader/nbgrader/docs/source/user_guide/creating_and_grading_assignments_nb.rst:179: WARNING: duplicate label manually-graded-task-cell-mark-scheme, other instance in /Users/jhamrick/project/nbgrader/nbgrader/docs/source/user_guide/creating_and_grading_assignments_jlab.rst
/Users/jhamrick/project/nbgrader/nbgrader/docs/source/user_guide/creating_and_grading_assignments_nb.rst:191: WARNING: duplicate label autograded-answer-cells, other instance in /Users/jhamrick/project/nbgrader/nbgrader/docs/source/user_guide/creating_and_grading_assignments_jlab.rst
/Users/jhamrick/project/nbgrader/nbgrader/docs/source/user_guide/creating_and_grading_assignments_nb.rst:240: WARNING: duplicate label autograder-tests-cell-hidden-tests, other instance in /Users/jhamrick/project/nbgrader/nbgrader/docs/source/user_guide/creating_and_grading_assignments_jlab.rst
/Users/jhamrick/project/nbgrader/nbgrader/docs/source/user_guide/creating_and_grading_assignments_nb.rst:267: WARNING: duplicate label read-only-cells, other instance in /Users/jhamrick/project/nbgrader/nbgrader/docs/source/user_guide/creating_and_grading_assignments_jlab.rst
/Users/jhamrick/project/nbgrader/nbgrader/docs/source/user_guide/creating_and_grading_assignments_nb.rst:342: WARNING: duplicate label assign-and-release-an-assignment, other instance in /Users/jhamrick/project/nbgrader/nbgrader/docs/source/user_guide/creating_and_grading_assignments_jlab.rst
/Users/jhamrick/project/nbgrader/nbgrader/docs/source/user_guide/creating_and_grading_assignments_nb.rst:494: WARNING: duplicate label autograde-assignments, other instance in /Users/jhamrick/project/nbgrader/nbgrader/docs/source/user_guide/creating_and_grading_assignments_jlab.rst
/Users/jhamrick/project/nbgrader/nbgrader/docs/source/user_guide/managing_assignment_files_nb.rst:145: WARNING: duplicate label fetching-assignments, other instance in /Users/jhamrick/project/nbgrader/nbgrader/docs/source/user_guide/managing_assignment_files_jlab.rst
/Users/jhamrick/project/nbgrader/nbgrader/docs/source/configuration/jupyterhub_config.rst:7: WARNING: unknown document: /user_guide/creating_and_grading_assignments
/Users/jhamrick/project/nbgrader/nbgrader/docs/source/configuration/jupyterhub_config.rst:10: WARNING: unknown document: /user_guide/managing_assignment_files
/Users/jhamrick/project/nbgrader/nbgrader/docs/source/configuration/jupyterhub_config.rst:23: WARNING: unknown document: /user_guide/managing_assignment_files
/Users/jhamrick/project/nbgrader/nbgrader/docs/source/configuration/jupyterhub_config.rst:23: WARNING: unknown document: /user_guide/creating_and_grading_assignments
/Users/jhamrick/project/nbgrader/nbgrader/docs/source/configuration/student_version.rst:7: WARNING: unknown document: /user_guide/creating_and_grading_assignments
/Users/jhamrick/project/nbgrader/nbgrader/docs/source/user_guide/advanced.rst:25: WARNING: unknown document: managing_assignment_files
/Users/jhamrick/project/nbgrader/nbgrader/docs/source/user_guide/creating_and_grading_assignments_jlab.rst:448: WARNING: unknown document: managing_assignment_files
/Users/jhamrick/project/nbgrader/nbgrader/docs/source/user_guide/creating_and_grading_assignments_jlab.rst:462: WARNING: unknown document: managing_assignment_files
/Users/jhamrick/project/nbgrader/nbgrader/docs/source/user_guide/creating_and_grading_assignments_jlab.rst:464: WARNING: unknown document: managing_assignment_files
/Users/jhamrick/project/nbgrader/nbgrader/docs/source/user_guide/creating_and_grading_assignments_nb.rst:449: WARNING: unknown document: managing_assignment_files
/Users/jhamrick/project/nbgrader/nbgrader/docs/source/user_guide/creating_and_grading_assignments_nb.rst:463: WARNING: unknown document: managing_assignment_files
/Users/jhamrick/project/nbgrader/nbgrader/docs/source/user_guide/creating_and_grading_assignments_nb.rst:465: WARNING: unknown document: managing_assignment_files
/Users/jhamrick/project/nbgrader/nbgrader/docs/source/user_guide/faq.rst:75: WARNING: unknown document: managing_assignment_files
/Users/jhamrick/project/nbgrader/nbgrader/docs/source/user_guide/managing_assignment_files_jlab.rst:65: WARNING: unknown document: creating_and_grading_assignments
/Users/jhamrick/project/nbgrader/nbgrader/docs/source/user_guide/managing_assignment_files_jlab.rst:465: WARNING: unknown document: creating_and_grading_assignments
/Users/jhamrick/project/nbgrader/nbgrader/docs/source/user_guide/managing_assignment_files_nb.rst:65: WARNING: unknown document: creating_and_grading_assignments
/Users/jhamrick/project/nbgrader/nbgrader/docs/source/user_guide/managing_assignment_files_nb.rst:457: WARNING: unknown document: creating_and_grading_assignments

@@ -0,0 +1,1149 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I'm not sure about the duplication of the documentation here. On the one hand, I get that there are differences between this and the classic interface and it might make things confusing to describe both interfaces in the same place. On the other hand though, now a lot of the same stuff about nbgrader is now duplicated, which will make maintaining the documentation more difficult. Maybe it's fine for now how you've done it, but perhaps you can open an issue to look into rewriting the documentation to reduce the duplication?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree.
As this branch is not supposed to keep compatibility with Classic Notebook, maybe we should drop the documentation on this for the next release ?

Copy link
Member

Choose a reason for hiding this comment

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

Sounds ok to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The documentation has been updated in db5eb6a

@@ -0,0 +1,911 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

Same comment here as above about duplication in the documentation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The documentation has been updated in db5eb6a

tasks.py Outdated
run(cmd)

env = os.environ.copy()
if args.group != 'labextensions':
Copy link
Member

Choose a reason for hiding this comment

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

Is this logic correct for when the group is 'all'?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, fixed in ca01cd6

@jhamrick
Copy link
Member

jhamrick commented Jun 6, 2022

One more comment too, after installing and building the docs git status looks like this:

$ git status
On branch jupyterlab_extensions
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   yarn.lock

Untracked files:
  (use "git add <file>..." to include in what will be committed)
	nbgrader/docs/source/user_guide/creating_and_grading_assignments_jlab.rst
	nbgrader/docs/source/user_guide/creating_and_grading_assignments_nb.rst
	nbgrader/docs/source/user_guide/managing_assignment_files_jlab.rst
	nbgrader/docs/source/user_guide/managing_assignment_files_nb.rst
	nbgrader/labextension/
	tsconfig.tsbuildinfo

Can the untracked files be added to .gitignore? And perhaps yarn.lock shouldn't be committed if it changes when doing a development install?

@brichet
Copy link
Contributor Author

brichet commented Jun 6, 2022

Thanks @jhamrick for the feedback. I'll take a look at it as soon as possible.


jupyter labextension develop --overwrite .
Copy link
Member

Choose a reason for hiding this comment

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

This command will fail unless pip install has been run first, as I guess the pip command builds the lab extension. Perhaps it's worth stating this explicitly, to avoid confusion?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed in ca01cd6

@brichet
Copy link
Contributor Author

brichet commented Jun 7, 2022

  • In the assignment list, the formatting causes the status messages to be clipped on the left side. This is partially aesthetic, but if you get the error that the exchange is not setup correctly, it hides the "h" from "http" in the the provided documentation url which will definitely confuse some people. Can the formatting be adjusted to fix this so there's no clipping?

Fixed in 47fb9b9

@brichet
Copy link
Contributor Author

brichet commented Jun 7, 2022

  • The "Create Assignment" extension doesn't seem to work for me. It appears on the right hand side as in the screenshots, but when I click on it nothing happens.

Fixed in df462bd
The extension worked but the accordion did not open properly (width of 0px).

@brichet
Copy link
Contributor Author

brichet commented Jun 7, 2022

  • Validation output looks different depending on whether you run it from the assignment list extension or in the notebook itself (and both of these look different from how it appears when using the classic notebook, which shows identical output regardless of whether it's generated from the assignment list extension or from the notebook). When running validation from the notebook in lab, the errors are displayed with a pink background and are not scrollable:

Fixed in 5c52bd1 by using the same functions to validate Notebook and display modal, from Notebook itself and from Assignment list

@brichet
Copy link
Contributor Author

brichet commented Jun 7, 2022

  • Mathjax does not seem to render when viewing generated feedback in a tab in lab, but it does render when viewing the same file directly in the browser. Example in lab:

The formgrader frontend extension is essentially an iframe including the output of the formgrader server extension. It seems that the Mathjax problem is a security issue. There is a "Trust HTML" button in the upper left corner of the formgrader tab, which renders correctly. I have not yet found a way to trust it automatically.

@brichet
Copy link
Contributor Author

brichet commented Jun 7, 2022

  • When I run "Validate" on the instructor notebook through lab after having tried to activate the "Create Assignment" extension, I get the following error: "The nbgrader ID "null" has been used for more than one cell. Please make sure all grade cells have unique ids." This does not occur when running validate from the traditional notebook interface. It also does not occur if I haven't tried to use the "Create Assignment" extension.

I can't reproduce that behaviour.

@perllaghu
Copy link
Contributor

Menu items should not be added unless the extension is enabled... more specifically removed if the extension is disabled.

In detail:

In the docker build, I have:

# Enable common nbgrader stuff
RUN jupyter nbextension install --sys-prefix --py nbgrader \
    && jupyter nbextension enable --sys-prefix validate_assignment/main --section=tree \
    && jupyter serverextension enable --sys-prefix nbgrader.server_extensions.validate_assignment \
    && jupyter nbextension enable --sys-prefix assignment_list/main --section=tree \
    && jupyter serverextension enable --sys-prefix nbgrader.server_extensions.assignment_list

.... and then in the docker-entrypoint.sh script, I have:

if [ "${USER_ROLE}" == "Instructor" ]; then
    jupyter nbextension enable --sys-prefix create_assignment/main
    jupyter nbextension enable --sys-prefix formgrader/main --section=tree
    jupyter serverextension enable --sys-prefix nbgrader.server_extensions.formgrader
fi

This gives me the NBgrader menu in jupyterlab... however it gives me formgrader even if the user is not an instructor.

@brichet
Copy link
Contributor Author

brichet commented Jun 7, 2022

  • When building the docs, I get a bunch of warnings that should probably be addressed:

Fixed in ca01cd6

@brichet
Copy link
Contributor Author

brichet commented Jun 7, 2022

Can the untracked files be added to .gitignore? And perhaps yarn.lock shouldn't be committed if it changes when doing a development install?

fixed in ca01cd6

@jhamrick
Copy link
Member

jhamrick commented Jun 7, 2022

Great, now I can test "Create Assignment"---thanks! It looks good, except that I am unable to scroll down if there are many cells in the notebook. e.g in the screenshot below, you can see that the cell on the left can be scrolled into view, but its corresponding area in the extension is cut off, and I can't seem to make it scroll down:

image

I also am still seeing yarn.lock in the repo. As it gets regenerated and changed upon running pip install, I think perhaps it should just not be committed and then added to .gitignore? Or is there a reason it needs to be committed?

Regarding the error message about the null cell ids, what notebook are you testing with? I get it with both of the notebooks included from nbgrader quickstart.

@brichet
Copy link
Contributor Author

brichet commented Jun 14, 2022

I think there's a bit of a deeper problem here, which is how to run the formgrader as a service now---previously we were doing this by just running the singleuser server and providing access to it, which worked ok since we would just open the formgrader as a new browser tab. But since everything now lives within lab, I'm not sure that would work?

Opening the correct formgrader in a new JupyterLab tab should be OK, this is the URL of the formgrader to link to in the iframe.
The main problem with using formgrader as a service is being able to change the root directory, as the formgrader server extension runs in its own root directory, which is not the same as the jupyterlab instance.
I wonder if the best way is to open a new browser tab with a new instance of jupyterlab (if possible). This means that running a local formgrader would work as now (all in jupyterlab), but when it's a service formgrader it would open a new jupyterlab instance in a new tab (with the correct root directory).

@brichet
Copy link
Contributor Author

brichet commented Jun 15, 2022

To reproduce these issues, you can run the demos linked above---you'll need to patch in #1597 (until it's merged) as well as the following patch: jupyterlab-patch.txt

@jhamrick Thanks for the commands to reproduce.
As explain above, I modified the course list to be able to open services formgrader. As the root directory changes between the user jupyter server and the service jupyter server, we use two different jupyterlab instances.

  • If the kind of formgrader is ' local' then it opens the formgrader tab in jupyterlab.
  • Else it opens a new jupyterlab instance in a new browser tab.

This new jupyter lab instance can open automatically the formgrader tab by applying that patch next to the 2 previous (#1597 and jupyterlab-patch.txt) :

EDIT : that patch was for docker installation. Otherwise the file to copy is demos/formgrader_workspace.json.

diff --git a/demos/utils.sh b/demos/utils.sh
index 64211058..9d3fe3ba 100644
--- a/demos/utils.sh
+++ b/demos/utils.sh
@@ -42,6 +42,10 @@ setup_nbgrader () {
     ${runas} mkdir -p "${HOME}/.jupyter"
     ${runas} cp "${config}" "${HOME}/.jupyter/nbgrader_config.py"
     ${runas} chown "${USER}:${USER}" "${HOME}/.jupyter/nbgrader_config.py"
+
+    cp "/root/formgrader_workspace.json" "${HOME}/formgrader_workspace.json"
+    chown "${USER}:${USER}" "${HOME}/formgrader_workspace.json"
+    ${runas} jupyter lab workspaces import "${HOME}/formgrader_workspace.json"
 }
 
 setup_jupyterhub () {

@jhamrick
Copy link
Member

Ah fantastic! Thanks for the patch, this looks great.

So I think the remaining two issues are:

  1. Extensions being enabled, even after having been disabled.
  2. The yarn.lock file getting regenerated when doing a development install. I think (2) is a problem as it will definitely cause merge conflicts and contributors will unknowingly try to commit it when making PRs with unrelated changes. But I also understand it's supposed to be committed into the repo. Is there any way we can avoid it getting changed on every install? Maybe we could have an explicit command to update it, rather than it getting changed automatically?

Once those issues are resolved I think I am happy to merge this and continue iterating on main as more people start to test it out!

@jtpio
Copy link
Member

jtpio commented Jun 15, 2022

2. The yarn.lock file getting regenerated when doing a development install. I think (2) is a problem as it will definitely cause merge conflicts and contributors will unknowingly try to commit it when making PRs with unrelated changes. But I also understand it's supposed to be committed into the repo. Is there any way we can avoid it getting changed on every install? Maybe we could have an explicit command to update it, rather than it getting changed automatically?

Normally it should not changed, unless new dependencies are added to the package.json. Probably running jlpm locally one more time will update yarn.lock. The changes should then be committed and pushed to the branch.

@brichet
Copy link
Contributor Author

brichet commented Jun 15, 2022

  1. Extensions being enabled, even after having been disabled.

It's working on my side with other commands.
To disable extensions at sys_prefix level, I think the command jupyter labextension disable --level=sys_prefix nbgrader create confusion in that case (maybe due to the way the extensions are built). If you repeat commands for each extension individually it works fine.

To enable extensions at user level, it may be an issue on Jupyterlab, depending on what was the initial goal. I'll try to explain what I understood, but I may be wrong.
If you disable an extension, it create a file page_config.json (at user, sys_prefix or system level). And the files will be read in that order, so if an extension is enabled or disabled explicitly at user level, it will not look for another file at sys_prefix or system level.
But it also seems that enabling an extension at a level where it has not been explicitly disabled does nothing (no file is created).
In our case, we disable extensions at sys_prefix level, that's OK. Then we try to enable it at user level, which does nothing. So the extension remain disabled. A workaround is to disable it before enabling it at user level.

You can try applying this patch which works on my side :

diff --git a/demos/restart_demo.sh b/demos/restart_demo.sh
index bcc00420..abf76a43 100755
--- a/demos/restart_demo.sh
+++ b/demos/restart_demo.sh
@@ -58,7 +58,9 @@ install_nbgrader () {
     jupyter nbextension install --symlink --sys-prefix --py nbgrader --overwrite
     jupyter nbextension disable --sys-prefix --py nbgrader
     jupyter labextension develop --overwrite .
-    jupyter labextension disable --level=sys_prefix nbgrader
+    jupyter labextension disable --level=sys_prefix nbgrader/formgrader
+    jupyter labextension disable --level=sys_prefix nbgrader/course-list
+    jupyter labextension disable --level=sys_prefix nbgrader/create-assignment
     jupyter serverextension disable --sys-prefix --py nbgrader
 
     # Everybody gets the validate extension, however.
diff --git a/demos/utils.sh b/demos/utils.sh
index 9d3fe3ba..d3055659 100644
--- a/demos/utils.sh
+++ b/demos/utils.sh
@@ -70,7 +70,8 @@ enable_create_assignment () {
     local runas="sudo -u ${USER}"
 
     ${runas} jupyter nbextension enable --user create_assignment/main
-    # ${runas} jupyter labextension enable --level=user nbgrader/create-assignment
+    ${runas} jupyter labextension disable --level=user nbgrader/create-assignment
+    ${runas} jupyter labextension enable --level=user nbgrader/create-assignment
 }
 
 enable_formgrader () {
@@ -79,6 +80,7 @@ enable_formgrader () {
     local runas="sudo -u ${USER}"
 
     ${runas} jupyter nbextension enable --user formgrader/main --section=tree
+    ${runas} jupyter labextension disable --level=user nbgrader/formgrader
     ${runas} jupyter labextension enable --level=user nbgrader/formgrader
     ${runas} jupyter serverextension enable --user nbgrader.server_extensions.formgrader
 }
@@ -89,6 +91,7 @@ enable_assignment_list () {
     local runas="sudo -u ${USER}"
 
     ${runas} jupyter nbextension enable --user assignment_list/main --section=tree
+    ${runas} jupyter labextension disable --level=user nbgrader/assignment-list
     ${runas} jupyter labextension enable --level=user nbgrader/assignment-list
     ${runas} jupyter serverextension enable --user nbgrader.server_extensions.assignment_list
 }
@@ -99,6 +102,7 @@ enable_course_list () {
     local runas="sudo -u ${USER}"
 
     ${runas} jupyter nbextension enable --user course_list/main --section=tree
+    ${runas} jupyter labextension disable --level=user nbgrader/course-list
     ${runas} jupyter labextension enable --level=user nbgrader/course-list
     ${runas} jupyter serverextension enable --user nbgrader.server_extensions.course_list
 }

@jhamrick
Copy link
Member

Yeah, I find it strange that jupyter labextension disable --level=sys_prefix nbgrader doesn't disable all of the extensions---perhaps it's worth removing that from the documentation as it's confusing?

Otherwise your suggested changes look good and everything seems to be working with the demos now! 🎉

So I guess the final thing would be to rerun jlpm as suggested by @jtpio ?

Copy link
Member

@jhamrick jhamrick left a comment

Choose a reason for hiding this comment

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

Looks great. Thanks so much!!

@jhamrick jhamrick merged commit 66edc83 into jupyter:main Jun 16, 2022
@brichet
Copy link
Contributor Author

brichet commented Jun 16, 2022

Thanks @jhamrick @perllaghu @jtpio and @SylvainCorlay for testing and code review

@orboan
Copy link

orboan commented Jul 27, 2022

If anyone could take a look at this issue:

1645

Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Port the notebook extensions to JupyterLab
10 participants