-
Notifications
You must be signed in to change notification settings - Fork 317
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
Jupyterlab extensions #1588
Conversation
Awesome---thanks a lot for this! Will try to have a look sometime next week... |
Working on documentation I just realize that there are some problems with images being duplicated. |
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 |
I'm looking at this in a notebook, How does one access If I switch to the classic interface, they're there.... |
I will update the documentation soon. |
For traceability : |
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 In the lab UI, the |
Thanks @perllaghu for your feedback. |
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... |
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. |
In a separate thought-stream: did you consider adding the I think these are correct - it's part of the typescript in
and/or
(I was playing with something different over at https://github.com/edina/noteable-htmlProject ) |
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 :) |
I added menu entries for |
There was a problem hiding this 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?
-
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.
- 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:
Same file in browser:
- 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:
When running validation from the assignment list extension in lab, the errors do not have a pink background and are displayed in scrollable boxes:
Classic notebook:
- 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
nbgrader/docs/source/contributor_guide/installation_developer.rst
Outdated
Show resolved
Hide resolved
nbgrader/docs/source/contributor_guide/installation_developer.rst
Outdated
Show resolved
Hide resolved
@@ -0,0 +1,1149 @@ | |||
{ |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds ok to me.
There was a problem hiding this comment.
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 @@ | |||
{ |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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': |
There was a problem hiding this comment.
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'?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, fixed in ca01cd6
One more comment too, after installing and building the docs
Can the untracked files be added to |
Thanks @jhamrick for the feedback. I'll take a look at it as soon as possible. |
|
||
jupyter labextension develop --overwrite . |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed in ca01cd6
Fixed in 47fb9b9 |
Fixed in df462bd |
Fixed in 5c52bd1 by using the same functions to validate Notebook and display modal, from Notebook itself and from |
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. |
I can't reproduce that behaviour. |
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:
.... and then in the
This gives me the |
Fixed in ca01cd6 |
fixed in ca01cd6 |
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: I also am still seeing 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 |
…ion warnings and (3) fix installation using tasks.py
Co-authored-by: Jeremy Tuloup <jeremy.tuloup@gmail.com>
…tensions, add scroll bar and padding in assignment and course list extensions, and fix create assignment extension
…n without channel, using tbump
Opening the correct formgrader in a new JupyterLab tab should be OK, this is the URL of the formgrader to link to in the |
@jhamrick Thanks for the commands to reproduce.
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
|
Ah fantastic! Thanks for the patch, this looks great. So I think the remaining two issues are:
Once those issues are resolved I think I am happy to merge this and continue iterating on |
Normally it should not changed, unless new dependencies are added to the |
It's working on my side with other commands. 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. You can try applying this patch which works on my side :
|
Yeah, I find it strange that 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 |
There was a problem hiding this 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!!
Thanks @jhamrick @perllaghu @jtpio and @SylvainCorlay for testing and code review |
If anyone could take a look at this issue: Thank you |
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