From cb491078055901113f68e21320f7e480c1f1ff42 Mon Sep 17 00:00:00 2001 From: Nicolas Brichet Date: Wed, 24 Jan 2024 16:37:36 +0100 Subject: [PATCH 01/12] Allow formgrader to update its config from the current directory in lab --- nbgrader/apps/baseapp.py | 4 +- nbgrader/server_extensions/formgrader/base.py | 4 +- .../formgrader/formgrader.py | 5 +- .../server_extensions/formgrader/handlers.py | 23 ++++-- package.json | 1 + src/index.ts | 71 +++++++++++++------ yarn.lock | 3 +- 7 files changed, 76 insertions(+), 35 deletions(-) diff --git a/nbgrader/apps/baseapp.py b/nbgrader/apps/baseapp.py index 48585fbf7..37d503eb2 100644 --- a/nbgrader/apps/baseapp.py +++ b/nbgrader/apps/baseapp.py @@ -313,10 +313,10 @@ def excepthook(self, etype, evalue, tb): format_excepthook(etype, evalue, tb) @catch_config_error - def initialize(self, argv: TypingList[str] = None) -> None: + def initialize(self, argv: TypingList[str] = None, root: str = '') -> None: self.update_config(self.build_extra_config()) self.init_syspath() - self.coursedir = CourseDirectory(parent=self) + self.coursedir = CourseDirectory(parent=self, root=root) super(NbGrader, self).initialize(argv) # load config that is in the coursedir directory diff --git a/nbgrader/server_extensions/formgrader/base.py b/nbgrader/server_extensions/formgrader/base.py index d5bf2ec4d..b6854ea24 100644 --- a/nbgrader/server_extensions/formgrader/base.py +++ b/nbgrader/server_extensions/formgrader/base.py @@ -16,7 +16,7 @@ def base_url(self): @property def db_url(self): - return self.settings['nbgrader_coursedir'].db_url + return self.coursedir.db_url @property def url_prefix(self): @@ -24,7 +24,7 @@ def url_prefix(self): @property def coursedir(self): - return self.settings['nbgrader_coursedir'] + return self.settings['nbgrader_formgrader'].coursedir @property def authenticator(self): diff --git a/nbgrader/server_extensions/formgrader/formgrader.py b/nbgrader/server_extensions/formgrader/formgrader.py index ff032db55..b8c2d11ec 100644 --- a/nbgrader/server_extensions/formgrader/formgrader.py +++ b/nbgrader/server_extensions/formgrader/formgrader.py @@ -33,10 +33,8 @@ def url_prefix(self): return relpath def load_config(self): - paths = jupyter_config_path() - paths.insert(0, os.getcwd()) app = NbGrader() - app.config_file_paths.append(paths) + app.config_dir = self.config_dir app.load_config_file() return app.config @@ -72,7 +70,6 @@ def init_tornado_settings(self, webapp): # Configure the formgrader settings tornado_settings = dict( nbgrader_formgrader=self, - nbgrader_coursedir=self.coursedir, nbgrader_authenticator=self.authenticator, nbgrader_exporter=HTMLExporter(config=self.config), nbgrader_gradebook=None, diff --git a/nbgrader/server_extensions/formgrader/handlers.py b/nbgrader/server_extensions/formgrader/handlers.py index fb65eea8e..7b78f9260 100644 --- a/nbgrader/server_extensions/formgrader/handlers.py +++ b/nbgrader/server_extensions/formgrader/handlers.py @@ -8,19 +8,34 @@ from ...api import MissingEntry +class FormgraderHandler(BaseHandler): + @web.authenticated + @check_xsrf + @check_notebook_dir + def get(self): + formgrader = self.settings['nbgrader_formgrader'] + path = self.get_argument('path', os.getcwd()) + path = os.path.abspath(path) + formgrader.config_dir = path + formgrader.load_config_file() + formgrader.initialize([], root=path) + self.redirect('/formgrader/manage_assignments/') + + class ManageAssignmentsHandler(BaseHandler): @web.authenticated @check_xsrf @check_notebook_dir def get(self): + api = self.api html = self.render( "manage_assignments.tpl", url_prefix=self.url_prefix, base_url=self.base_url, windows=(sys.prefix == 'win32'), - course_id=self.api.course_id, - exchange=self.api.exchange_root, - exchange_missing=self.api.exchange_missing) + course_id=api.course_id, + exchange=api.exchange_root, + exchange_missing=api.exchange_missing) self.write(html) @@ -282,7 +297,7 @@ def prepare(self): _navigation_regex = r"(?Pnext_incorrect|prev_incorrect|next|prev)" default_handlers = [ - (r"/formgrader/?", ManageAssignmentsHandler), + (r"/formgrader/?", FormgraderHandler), (r"/formgrader/manage_assignments/?", ManageAssignmentsHandler), (r"/formgrader/manage_submissions/([^/]+)/?", ManageSubmissionsHandler), diff --git a/package.json b/package.json index 31f45217c..60fff0c52 100644 --- a/package.json +++ b/package.json @@ -51,6 +51,7 @@ "@jupyterlab/cells": "^4.2.0", "@jupyterlab/coreutils": "^6.2.0", "@jupyterlab/docregistry": "^4.2.0", + "@jupyterlab/filebrowser": "^4.2.0", "@jupyterlab/mainmenu": "^4.2.0", "@jupyterlab/nbformat": "^4.2.0", "@jupyterlab/notebook": "^4.2.0", diff --git a/src/index.ts b/src/index.ts index 3f500a9cb..4f9e7f7ef 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,6 +1,7 @@ import { ILabShell, ILayoutRestorer, IRouter, JupyterFrontEnd, JupyterFrontEndPlugin } from "@jupyterlab/application"; import { ICommandPalette, MainAreaWidget, WidgetTracker } from "@jupyterlab/apputils"; import { PageConfig, URLExt } from "@jupyterlab/coreutils"; +import { IDefaultFileBrowser } from "@jupyterlab/filebrowser"; import { IMainMenu } from '@jupyterlab/mainmenu'; import { INotebookTracker } from "@jupyterlab/notebook"; import { ServerConnection } from "@jupyterlab/services"; @@ -33,6 +34,7 @@ export const commandIDs = { openAssignmentsList: 'nbgrader:open-assignment-list', openCoursesList: 'nbgrader:open-course-list', openFormgrader: 'nbgrader:open-formgrader', + openFormgraderLocal: 'nbgrader:open-formgrader-local', openCreateAssignment: 'nbgrader:open-create-assignment' } @@ -75,6 +77,7 @@ const availableExtensionsManager: JupyterFrontEndPlugin = { nbgraderMenu.addItem({ command: commandIDs.openAssignmentsList }); nbgraderMenu.addItem({ command: commandIDs.openCoursesList }); nbgraderMenu.addItem({ command: commandIDs.openFormgrader }); + nbgraderMenu.addItem({ command: commandIDs.openFormgraderLocal }); if (palette) { palette.addItem({ @@ -89,6 +92,10 @@ const availableExtensionsManager: JupyterFrontEndPlugin = { command: commandIDs.openFormgrader, category: 'nbgrader' }); + palette.addItem({ + command: commandIDs.openFormgraderLocal, + category: 'nbgrader' + }); } } @@ -239,9 +246,10 @@ const courseListExtension: JupyterFrontEndPlugin = { const formgraderExtension: JupyterFrontEndPlugin = { id: pluginIDs.formgrader, autoStart: true, - optional: [ILayoutRestorer, INotebookTree, IRouter], + optional: [IDefaultFileBrowser, ILayoutRestorer, INotebookTree, IRouter], activate: ( app: JupyterFrontEnd, + defaultFileBrowser: IDefaultFileBrowser, restorer: ILayoutRestorer | null, notebookTree: INotebookTree | null, router: IRouter | null @@ -254,39 +262,58 @@ const formgraderExtension: JupyterFrontEndPlugin = { namespace: 'nbgrader-formgrader' }); - app.commands.addCommand(commandIDs.openFormgrader, { - label: 'Formgrader', - execute: args => { + const openFormgrader = (url: string) => { if (!widget || widget.isDisposed) { - const settings = ServerConnection.makeSettings(); - const url = (args.url as string) || URLExt.join(settings.baseUrl, "formgrader"); - const content = new FormgraderWidget(app, url); widget = new MainAreaWidget({content}); widget.id = 'formgrader'; widget.title.label = 'Formgrader'; widget.title.closable = true; - } + } - if (!tracker.has(widget)) { - // Track the state of the widget for later restoration - tracker.add(widget); - } + if (!tracker.has(widget)) { + // Track the state of the widget for later restoration + tracker.add(widget); + } - // Attach the widget to the main area if it's not there - if (notebookTree){ - if (!widget.isAttached){ - notebookTree.addWidget(widget); - } - notebookTree.currentWidget = widget; - } else if (!widget.isAttached) { - app.shell.add(widget, 'main'); + // Attach the widget to the main area if it's not there + if (notebookTree){ + if (!widget.isAttached){ + notebookTree.addWidget(widget); } + notebookTree.currentWidget = widget; + } else if (!widget.isAttached) { + app.shell.add(widget, 'main'); + } - widget.content.update(); + widget.content.update(); - app.shell.activateById(widget.id); + app.shell.activateById(widget.id); + } + + app.commands.addCommand(commandIDs.openFormgrader, { + label: 'Formgrader', + execute: args => { + const settings = ServerConnection.makeSettings(); + let url = (args.url as string) || URLExt.join(settings.baseUrl, 'formgrader'); + openFormgrader(url); + } + }); + + app.commands.addCommand(commandIDs.openFormgraderLocal, { + label: 'Formgrader (local)', + execute: args => { + let path = '' + if (defaultFileBrowser) { + path = encodeURIComponent(defaultFileBrowser.model.path); + } + const settings = ServerConnection.makeSettings(); + let url = (args.url as string) || URLExt.join(settings.baseUrl, 'formgrader'); + if (path) { + url += `?path=${path}`; + } + openFormgrader(url); } }); diff --git a/yarn.lock b/yarn.lock index e2062f2c3..5e2f27f56 100644 --- a/yarn.lock +++ b/yarn.lock @@ -504,6 +504,7 @@ __metadata: "@jupyterlab/cells": ^4.2.0 "@jupyterlab/coreutils": ^6.2.0 "@jupyterlab/docregistry": ^4.2.0 + "@jupyterlab/filebrowser": ^4.2.0 "@jupyterlab/galata": ^5.2.0 "@jupyterlab/mainmenu": ^4.2.0 "@jupyterlab/nbformat": ^4.2.0 @@ -936,7 +937,7 @@ __metadata: languageName: node linkType: hard -"@jupyterlab/filebrowser@npm:^4.2.2, @jupyterlab/filebrowser@npm:~4.2.0": +"@jupyterlab/filebrowser@npm:^4.2.0, @jupyterlab/filebrowser@npm:^4.2.2, @jupyterlab/filebrowser@npm:~4.2.0": version: 4.2.2 resolution: "@jupyterlab/filebrowser@npm:4.2.2" dependencies: From 76718cf1ea52632080f57526cbe75bc47c0e1a95 Mon Sep 17 00:00:00 2001 From: Nicolas Brichet Date: Wed, 24 Jan 2024 17:54:43 +0100 Subject: [PATCH 02/12] Restore the config to avoid confusion --- nbgrader/server_extensions/formgrader/handlers.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/nbgrader/server_extensions/formgrader/handlers.py b/nbgrader/server_extensions/formgrader/handlers.py index 7b78f9260..d2e066905 100644 --- a/nbgrader/server_extensions/formgrader/handlers.py +++ b/nbgrader/server_extensions/formgrader/handlers.py @@ -3,6 +3,7 @@ import sys from tornado import web +from traitlets.config.loader import Config from .base import BaseHandler, check_xsrf, check_notebook_dir from ...api import MissingEntry @@ -16,8 +17,8 @@ def get(self): formgrader = self.settings['nbgrader_formgrader'] path = self.get_argument('path', os.getcwd()) path = os.path.abspath(path) + formgrader.config = Config() formgrader.config_dir = path - formgrader.load_config_file() formgrader.initialize([], root=path) self.redirect('/formgrader/manage_assignments/') From b9b8825e90fdcc51ab077212df4c47500149ebd9 Mon Sep 17 00:00:00 2001 From: Nicolas Brichet Date: Thu, 25 Jan 2024 16:01:35 +0100 Subject: [PATCH 03/12] Initialize configuration only if necessary --- nbgrader/apps/baseapp.py | 5 ++++- .../formgrader/formgrader.py | 3 ++- .../server_extensions/formgrader/handlers.py | 19 +++++++++++++------ 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/nbgrader/apps/baseapp.py b/nbgrader/apps/baseapp.py index 37d503eb2..d468f8212 100644 --- a/nbgrader/apps/baseapp.py +++ b/nbgrader/apps/baseapp.py @@ -316,7 +316,10 @@ def excepthook(self, etype, evalue, tb): def initialize(self, argv: TypingList[str] = None, root: str = '') -> None: self.update_config(self.build_extra_config()) self.init_syspath() - self.coursedir = CourseDirectory(parent=self, root=root) + if root: + self.coursedir = CourseDirectory(parent=self, root=root) + else: + self.coursedir = CourseDirectory(parent=self) super(NbGrader, self).initialize(argv) # load config that is in the coursedir directory diff --git a/nbgrader/server_extensions/formgrader/formgrader.py b/nbgrader/server_extensions/formgrader/formgrader.py index b8c2d11ec..729e348da 100644 --- a/nbgrader/server_extensions/formgrader/formgrader.py +++ b/nbgrader/server_extensions/formgrader/formgrader.py @@ -75,7 +75,8 @@ def init_tornado_settings(self, webapp): nbgrader_gradebook=None, nbgrader_db_url=self.coursedir.db_url, nbgrader_jinja2_env=jinja_env, - nbgrader_bad_setup=nbgrader_bad_setup + nbgrader_bad_setup=nbgrader_bad_setup, + initial_config=self.config ) webapp.settings.update(tornado_settings) diff --git a/nbgrader/server_extensions/formgrader/handlers.py b/nbgrader/server_extensions/formgrader/handlers.py index d2e066905..b49b1c219 100644 --- a/nbgrader/server_extensions/formgrader/handlers.py +++ b/nbgrader/server_extensions/formgrader/handlers.py @@ -3,6 +3,7 @@ import sys from tornado import web +from jupyter_core.paths import jupyter_config_dir from traitlets.config.loader import Config from .base import BaseHandler, check_xsrf, check_notebook_dir @@ -15,12 +16,18 @@ class FormgraderHandler(BaseHandler): @check_notebook_dir def get(self): formgrader = self.settings['nbgrader_formgrader'] - path = self.get_argument('path', os.getcwd()) - path = os.path.abspath(path) - formgrader.config = Config() - formgrader.config_dir = path - formgrader.initialize([], root=path) - self.redirect('/formgrader/manage_assignments/') + path = self.get_argument('path', '') + if path: + path = os.path.abspath(path) + formgrader.config = Config() + formgrader.config_dir = path + formgrader.initialize([], root=path) + else: + if formgrader.config != self.settings['initial_config']: + formgrader.config = self.settings['initial_config'] + formgrader.config_dir = jupyter_config_dir() + formgrader.initialize([]) + self.redirect(f"{self.base_url}/formgrader/manage_assignments/") class ManageAssignmentsHandler(BaseHandler): From 519f5a572a561f99fa50849f3e5072f42d4ea5dd Mon Sep 17 00:00:00 2001 From: Nicolas Brichet Date: Thu, 25 Jan 2024 20:55:22 +0100 Subject: [PATCH 04/12] Add a debuf flag to FormgraderExtension to display the current configuration in formgrader UI --- .../server_extensions/formgrader/formgrader.py | 15 +++++++++++++-- .../server_extensions/formgrader/handlers.py | 13 ++++++++++++- .../formgrader/templates/manage_assignments.tpl | 16 ++++++++++++++++ 3 files changed, 41 insertions(+), 3 deletions(-) diff --git a/nbgrader/server_extensions/formgrader/formgrader.py b/nbgrader/server_extensions/formgrader/formgrader.py index 729e348da..bbc83da06 100644 --- a/nbgrader/server_extensions/formgrader/formgrader.py +++ b/nbgrader/server_extensions/formgrader/formgrader.py @@ -1,13 +1,13 @@ # coding: utf-8 import os +from textwrap import dedent from nbconvert.exporters import HTMLExporter -from traitlets import default +from traitlets import Bool, default from tornado import web from jinja2 import Environment, FileSystemLoader from jupyter_server.utils import url_path_join as ujoin -from jupyter_core.paths import jupyter_config_path from . import handlers, apihandlers from ...apps.baseapp import NbGrader @@ -18,6 +18,17 @@ class FormgradeExtension(NbGrader): name = u'formgrade' description = u'Grade a notebook using an HTML form' + debug = Bool( + False, + help=dedent( + """ + Whether to display the loaded configuration in the 'Formgrader -> + Manage Assignments' panel. This can help debugging some misconfiguration + when using several files. + """ + ) + ).tag(config=True) + @property def root_dir(self): return self._root_dir diff --git a/nbgrader/server_extensions/formgrader/handlers.py b/nbgrader/server_extensions/formgrader/handlers.py index b49b1c219..45e7f38bc 100644 --- a/nbgrader/server_extensions/formgrader/handlers.py +++ b/nbgrader/server_extensions/formgrader/handlers.py @@ -1,6 +1,7 @@ import os import re import sys +import json from tornado import web from jupyter_core.paths import jupyter_config_dir @@ -35,6 +36,15 @@ class ManageAssignmentsHandler(BaseHandler): @check_xsrf @check_notebook_dir def get(self): + formgrader = self.settings['nbgrader_formgrader'] + current_config = {} + if formgrader.debug: + try: + current_config = json.dumps(formgrader.config, indent=2) + except TypeError: + current_config = formgrader.config + self.log.warn("Formgrader config is not serializable") + api = self.api html = self.render( "manage_assignments.tpl", @@ -43,7 +53,8 @@ def get(self): windows=(sys.prefix == 'win32'), course_id=api.course_id, exchange=api.exchange_root, - exchange_missing=api.exchange_missing) + exchange_missing=api.exchange_missing, + current_config= current_config) self.write(html) diff --git a/nbgrader/server_extensions/formgrader/templates/manage_assignments.tpl b/nbgrader/server_extensions/formgrader/templates/manage_assignments.tpl index 51b6969f8..55c1a6adf 100644 --- a/nbgrader/server_extensions/formgrader/templates/manage_assignments.tpl +++ b/nbgrader/server_extensions/formgrader/templates/manage_assignments.tpl @@ -55,6 +55,22 @@ Manage Assignments +{% if current_config %} +
+
+ +
+
{{ current_config }}
+
+
+
+{% endif %} {% if windows %}
Windows operating system detected. Please note that the "release" and "collect" From 827cb37ee95d5ed21570abb013853b508bc37b40 Mon Sep 17 00:00:00 2001 From: Nicolas Brichet Date: Thu, 25 Jan 2024 21:26:05 +0100 Subject: [PATCH 05/12] Add a setting to enable/disable the local formgrader --- package.json | 3 ++- schema/formgrader.json | 14 ++++++++++++++ src/index.ts | 38 ++++++++++++++++++++++++++++++++++++-- 3 files changed, 52 insertions(+), 3 deletions(-) create mode 100644 schema/formgrader.json diff --git a/package.json b/package.json index 60fff0c52..559dba844 100644 --- a/package.json +++ b/package.json @@ -107,6 +107,7 @@ } }, "extension": true, - "outputDir": "nbgrader/labextension" + "outputDir": "nbgrader/labextension", + "schemaDir": "schema" } } diff --git a/schema/formgrader.json b/schema/formgrader.json new file mode 100644 index 000000000..e1e7c5010 --- /dev/null +++ b/schema/formgrader.json @@ -0,0 +1,14 @@ +{ + "title": "Nbgrader -> Formgrader", + "description": "Option of the formgrader extension.", + "properties": { + "local_config": { + "type": "boolean", + "title": "Allow local nbgrader config file", + "description": "This setting allow the use of a local config file for formgrader.", + "default": false + } + }, + "additionalProperties": false, + "type": "object" +} diff --git a/src/index.ts b/src/index.ts index 4f9e7f7ef..0c6f35484 100644 --- a/src/index.ts +++ b/src/index.ts @@ -4,6 +4,7 @@ import { PageConfig, URLExt } from "@jupyterlab/coreutils"; import { IDefaultFileBrowser } from "@jupyterlab/filebrowser"; import { IMainMenu } from '@jupyterlab/mainmenu'; import { INotebookTracker } from "@jupyterlab/notebook"; +import { ISettingRegistry } from "@jupyterlab/settingregistry"; import { ServerConnection } from "@jupyterlab/services"; import { INotebookShell } from "@jupyter-notebook/application"; import { INotebookTree } from "@jupyter-notebook/tree"; @@ -246,17 +247,26 @@ const courseListExtension: JupyterFrontEndPlugin = { const formgraderExtension: JupyterFrontEndPlugin = { id: pluginIDs.formgrader, autoStart: true, - optional: [IDefaultFileBrowser, ILayoutRestorer, INotebookTree, IRouter], + optional: [ + IDefaultFileBrowser, + ILayoutRestorer, + INotebookTree, + IRouter, + ISettingRegistry], activate: ( app: JupyterFrontEnd, defaultFileBrowser: IDefaultFileBrowser, restorer: ILayoutRestorer | null, notebookTree: INotebookTree | null, - router: IRouter | null + router: IRouter | null, + settings: ISettingRegistry | null ) => { // Declare a widget variable let widget: MainAreaWidget; + // Whether formgrader can load the local settings or not + let localConfig = false; + // Track the widget state let tracker = new WidgetTracker>({ namespace: 'nbgrader-formgrader' @@ -292,6 +302,7 @@ const formgraderExtension: JupyterFrontEndPlugin = { app.shell.activateById(widget.id); } + // Command to open formgrader app.commands.addCommand(commandIDs.openFormgrader, { label: 'Formgrader', execute: args => { @@ -301,8 +312,10 @@ const formgraderExtension: JupyterFrontEndPlugin = { } }); + // Command to open formgrader using local configuration file app.commands.addCommand(commandIDs.openFormgraderLocal, { label: 'Formgrader (local)', + isVisible: () => localConfig, execute: args => { let path = '' if (defaultFileBrowser) { @@ -317,6 +330,27 @@ const formgraderExtension: JupyterFrontEndPlugin = { } }); + /** + * Load the settings for this extension + */ + function loadSetting(setting: ISettingRegistry.ISettings): void { + // Read the settings and convert to the correct type + localConfig = setting.get('local_config').composite as boolean; + + // Notify the command that the setting has been reloaded + app.commands.notifyCommandChanged(commandIDs.openFormgraderLocal); + } + + // Wait for the application to be restored and + // for the settings for this plugin to be loaded + Promise.all([app.restored, settings.load(pluginIDs.formgrader)]) + .then(([, setting]) => { + // Read the settings + loadSetting(setting); + // Listen for your plugin setting changes + setting.changed.connect(loadSetting); + }); + // Open formgrader from URL. if (router) { const formgraderPattern = /(\?|&)formgrader=true/; From 3956219c4795d5abfe60a4908b31d4efdc0d88e5 Mon Sep 17 00:00:00 2001 From: Nicolas Brichet Date: Fri, 26 Jan 2024 11:29:40 +0100 Subject: [PATCH 06/12] Avoid loading CWD config when API is called too --- nbgrader/apps/baseapp.py | 10 +++++++--- nbgrader/server_extensions/formgrader/formgrader.py | 1 + nbgrader/server_extensions/formgrader/handlers.py | 2 ++ 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/nbgrader/apps/baseapp.py b/nbgrader/apps/baseapp.py index d468f8212..4d513c577 100644 --- a/nbgrader/apps/baseapp.py +++ b/nbgrader/apps/baseapp.py @@ -64,6 +64,8 @@ class NbGrader(JupyterApp): aliases = nbgrader_aliases flags = nbgrader_flags + load_cwd_config = True + _log_formatter_cls = LogFormatter @default("log_level") @@ -358,7 +360,8 @@ def load_config_file(self, **kwargs: Any) -> None: paths = [os.path.abspath("{}.py".format(self.config_file))] else: config_dir = self.config_file_paths.copy() - config_dir.insert(0, os.getcwd()) + if self.load_cwd_config: + config_dir.insert(0, os.getcwd()) paths = [os.path.join(x, "{}.py".format(self.config_file_name)) for x in config_dir] if not any(os.path.exists(x) for x in paths): @@ -366,8 +369,9 @@ def load_config_file(self, **kwargs: Any) -> None: super(NbGrader, self).load_config_file(**kwargs) - # Load also config from current working directory - super(JupyterApp, self).load_config_file(self.config_file_name, os.getcwd()) + if (self.load_cwd_config): + # Load also config from current working directory + super(JupyterApp, self).load_config_file(self.config_file_name, os.getcwd()) def start(self) -> None: super(NbGrader, self).start() diff --git a/nbgrader/server_extensions/formgrader/formgrader.py b/nbgrader/server_extensions/formgrader/formgrader.py index bbc83da06..bae37b56c 100644 --- a/nbgrader/server_extensions/formgrader/formgrader.py +++ b/nbgrader/server_extensions/formgrader/formgrader.py @@ -45,6 +45,7 @@ def url_prefix(self): def load_config(self): app = NbGrader() + app.load_cwd_config = self.load_cwd_config app.config_dir = self.config_dir app.load_config_file() diff --git a/nbgrader/server_extensions/formgrader/handlers.py b/nbgrader/server_extensions/formgrader/handlers.py index 45e7f38bc..1efd407d4 100644 --- a/nbgrader/server_extensions/formgrader/handlers.py +++ b/nbgrader/server_extensions/formgrader/handlers.py @@ -20,6 +20,7 @@ def get(self): path = self.get_argument('path', '') if path: path = os.path.abspath(path) + formgrader.load_cwd_config = False formgrader.config = Config() formgrader.config_dir = path formgrader.initialize([], root=path) @@ -28,6 +29,7 @@ def get(self): formgrader.config = self.settings['initial_config'] formgrader.config_dir = jupyter_config_dir() formgrader.initialize([]) + formgrader.load_cwd_config = True self.redirect(f"{self.base_url}/formgrader/manage_assignments/") From a492821bf306d62585ad699834b68dd79dc4cfb9 Mon Sep 17 00:00:00 2001 From: Nicolas Brichet Date: Fri, 26 Jan 2024 12:41:50 +0100 Subject: [PATCH 07/12] Fix UI tests --- nbgrader/server_extensions/formgrader/handlers.py | 2 +- nbgrader/tests/ui-tests/formgrader.spec.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/nbgrader/server_extensions/formgrader/handlers.py b/nbgrader/server_extensions/formgrader/handlers.py index 1efd407d4..fbe55c85a 100644 --- a/nbgrader/server_extensions/formgrader/handlers.py +++ b/nbgrader/server_extensions/formgrader/handlers.py @@ -30,7 +30,7 @@ def get(self): formgrader.config_dir = jupyter_config_dir() formgrader.initialize([]) formgrader.load_cwd_config = True - self.redirect(f"{self.base_url}/formgrader/manage_assignments/") + self.redirect(f"{self.base_url}/formgrader/manage_assignments") class ManageAssignmentsHandler(BaseHandler): diff --git a/nbgrader/tests/ui-tests/formgrader.spec.ts b/nbgrader/tests/ui-tests/formgrader.spec.ts index a16037962..9ef3d4229 100644 --- a/nbgrader/tests/ui-tests/formgrader.spec.ts +++ b/nbgrader/tests/ui-tests/formgrader.spec.ts @@ -353,7 +353,7 @@ test("Load manage assignments", async ({ page, baseURL, request, tmpPath }) => { const iframe = page.mainFrame().childFrames()[0]; await checkFormgraderBreadcrumbs(iframe, ["Assignments"]); - expect(iframe.url()).toBe(encodeURI(`${baseURL}/formgrader`)); + expect(iframe.url()).toBe(encodeURI(`${baseURL}/formgrader/manage_assignments`)); // expect the current path in tree tab to be the tmpPath. await switchTab(page, 'Files'); From 9eacfca01561201a1fc3d178047d966ae4bf4dec Mon Sep 17 00:00:00 2001 From: Nicolas Brichet Date: Mon, 4 Mar 2024 11:35:40 +0100 Subject: [PATCH 08/12] Update documentation --- nbgrader/docs/source/build_docs.py | 2 ++ .../source/configuration/nbgrader_config.rst | 28 ++++++++++++++++++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/nbgrader/docs/source/build_docs.py b/nbgrader/docs/source/build_docs.py index 33c3e2eac..2748f40b4 100644 --- a/nbgrader/docs/source/build_docs.py +++ b/nbgrader/docs/source/build_docs.py @@ -5,6 +5,7 @@ import shutil import sys import nbgrader.apps +import nbgrader.server_extensions.formgrader from textwrap import dedent from clear_docs import run, clear_notebooks @@ -92,6 +93,7 @@ def autogen_config(root): print('Generating example configuration file') config = nbgrader.apps.NbGraderApp().document_config_options() + config += nbgrader.server_extensions.formgrader.formgrader.FormgradeExtension().document_config_options() destination = os.path.join(root, 'configuration', 'config_options.rst') with open(destination, 'w') as f: f.write(header) diff --git a/nbgrader/docs/source/configuration/nbgrader_config.rst b/nbgrader/docs/source/configuration/nbgrader_config.rst index eb89c5b3e..068f1a3e2 100644 --- a/nbgrader/docs/source/configuration/nbgrader_config.rst +++ b/nbgrader/docs/source/configuration/nbgrader_config.rst @@ -65,8 +65,34 @@ For example, the ``nbgrader_config.py`` that the notebook knows about could be p Then you would additionally have a config file at ``/path/to/course/directory/nbgrader_config.py``. +Use Case 3: using config from a specific sub directory +------------------------------------------------------ -Use Case 3: nbgrader and JupyterHub +.. warning:: + + This option should not be used with a JupyterHub installation, as it modifies + certain objects in the running instance, and can probably prevent other users + from using *formgrader* correctly. If you have a JupyterHub installation, + you should use the settings described in the following section. + +You may need to use a dedicated configuration file for each course without configuring +JupyterHub for all courses. In this case, the config file used will be the one from the +current directory in the filebrowser panel, instead of the one from the directory where +the jupyter server started. + +This option is not enabled by default. It can be enabled by using the settings panel: +*Nbgrader -> Formgrader* and check *Allow local nbgrader config file*. + +A new item is displayed in the *nbgrader* menu (or in the command palette), to open +formgrader from the local director: *Formgrader (local)*. + +.. warning:: + + If paths are used in the configuration file, note that the root of the relative + paths will always be the directory where the jupyter server was started, and not + the directory containing the ``nbgrader_config.py`` file. + +Use Case 4: nbgrader and JupyterHub ----------------------------------- .. seealso:: From 82c7d519bea153242fe93d9b3939e1624606a344 Mon Sep 17 00:00:00 2001 From: Nicolas Brichet Date: Mon, 25 Mar 2024 15:28:50 +0100 Subject: [PATCH 09/12] Add a warning about using it with several users on the same Jupyterlab instance, and show the configuration by default in formgrader --- nbgrader/docs/source/configuration/nbgrader_config.rst | 4 ++-- nbgrader/server_extensions/formgrader/formgrader.py | 2 +- schema/formgrader.json | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/nbgrader/docs/source/configuration/nbgrader_config.rst b/nbgrader/docs/source/configuration/nbgrader_config.rst index 068f1a3e2..fdc733057 100644 --- a/nbgrader/docs/source/configuration/nbgrader_config.rst +++ b/nbgrader/docs/source/configuration/nbgrader_config.rst @@ -70,9 +70,9 @@ Use Case 3: using config from a specific sub directory .. warning:: - This option should not be used with a JupyterHub installation, as it modifies + This option should not be used with a multiuser Jupyterlab instance, as it modifies certain objects in the running instance, and can probably prevent other users - from using *formgrader* correctly. If you have a JupyterHub installation, + from using *formgrader* correctly. Also, if you have a JupyterHub installation, you should use the settings described in the following section. You may need to use a dedicated configuration file for each course without configuring diff --git a/nbgrader/server_extensions/formgrader/formgrader.py b/nbgrader/server_extensions/formgrader/formgrader.py index bae37b56c..55330f919 100644 --- a/nbgrader/server_extensions/formgrader/formgrader.py +++ b/nbgrader/server_extensions/formgrader/formgrader.py @@ -19,7 +19,7 @@ class FormgradeExtension(NbGrader): description = u'Grade a notebook using an HTML form' debug = Bool( - False, + True, help=dedent( """ Whether to display the loaded configuration in the 'Formgrader -> diff --git a/schema/formgrader.json b/schema/formgrader.json index e1e7c5010..5cdbfca24 100644 --- a/schema/formgrader.json +++ b/schema/formgrader.json @@ -5,7 +5,7 @@ "local_config": { "type": "boolean", "title": "Allow local nbgrader config file", - "description": "This setting allow the use of a local config file for formgrader.", + "description": "This setting allows the use of a local config file for formgrader.\nWARNING: using the local configuration file has consequences for the server, it should not be used if several users are using the same instance of Jupyterlab.", "default": false } }, From 9e791d65bfbb38245f41803172c672a2ffb022be Mon Sep 17 00:00:00 2001 From: Nicolas Brichet Date: Tue, 18 Jun 2024 13:55:24 +0200 Subject: [PATCH 10/12] Add tests on local formgrader and increase time for doc test --- nbgrader/tests/ui-tests/formgrader.spec.ts | 162 ++++++++++++++++++++- 1 file changed, 160 insertions(+), 2 deletions(-) diff --git a/nbgrader/tests/ui-tests/formgrader.spec.ts b/nbgrader/tests/ui-tests/formgrader.spec.ts index 9ef3d4229..03a1e239e 100644 --- a/nbgrader/tests/ui-tests/formgrader.spec.ts +++ b/nbgrader/tests/ui-tests/formgrader.spec.ts @@ -1,5 +1,5 @@ import { test as jupyterLabTest, galata, IJupyterLabPageFixture } from "@jupyterlab/galata"; -import { APIRequestContext, expect, Frame } from "@playwright/test"; +import { APIRequestContext, expect, Frame, Locator} from "@playwright/test"; import * as path from "path"; import * as os from "os"; import * as fs from "fs"; @@ -27,6 +27,7 @@ let mainPanelTabCount = 1; const baseTestUse = { tmpPath: tempPath, mockSettings: { + ...galata.DEFAULT_SETTINGS, '@jupyterlab/apputils-extension:notification': { fetchNews: 'false' } @@ -58,6 +59,14 @@ test.beforeEach(async ({ request, tmpPath }) => { const contents = galata.newContentsHelper(request); + if (await contents.fileExists("nbgrader_config.py")) { + await contents.deleteFile("nbgrader_config.py"); + } + await contents.uploadFile( + path.resolve(__dirname, "./files/nbgrader_config.py"), + "nbgrader_config.py" + ); + await contents.createDirectory(tmpPath); if (await contents.fileExists("nbgrader_config.py")){ @@ -91,6 +100,22 @@ test.afterEach(async ({ request, page, tmpPath }) => { await contents.deleteDirectory(tmpPath); }); +const openSettings = async (page: IJupyterLabPageFixture): Promise => { + await page.evaluate(async () => { + await window.jupyterapp.commands.execute('settingeditor:open'); + }); + + // Activate the settings tab, sometimes it does not automatically. + const settingsTab = page + .getByRole('main') + .locator('.lm-TabBar-tab[data-id="setting-editor"]'); + await settingsTab.click(); + await page.waitForCondition( + async () => (await settingsTab.getAttribute('aria-selected')) === 'true' + ); + return (await page.activity.getPanelLocator('Settings')) as Locator; +}; + /* * Create a nbgrader file system */ @@ -348,7 +373,6 @@ test("Load manage assignments", async ({ page, baseURL, request, tmpPath }) => { await createEnv(testDir, tmpPath, exchange_dir, cache_dir, isWindows); await addCourses(request, page, tmpPath); await openFormgrader(page); - // get formgrader iframe and check for breadcrumbs const iframe = page.mainFrame().childFrames()[0]; @@ -860,3 +884,137 @@ test("Switch views", async ({ page, baseURL, request, tmpPath }) => { } } }); + +/** + * Local Formgrader. + */ +test.describe('#localFormgrader', () => { + test("Should have formgrader settings", async ({ page, tmpPath }) => { + test.skip(isWindows, "This test does not work on Windows"); + + if (isNotebook) await page.goto(`tree/${tmpPath}`); + + const settings = await openSettings(page); + const formgraderSettings = settings.locator( + '.jp-PluginList-entry[data-id="@jupyter/nbgrader:formgrader"]' + ); + await expect(formgraderSettings).toBeVisible(); + + await formgraderSettings.click(); + const settingsList = settings.locator('.jp-SettingsPanel fieldset > .form-group'); + await expect(settingsList).toHaveCount(1); + await expect( + settingsList.locator('input').first() + ).toHaveAttribute('type', 'checkbox'); + await expect( + settingsList.locator('input').first() + ).not.toBeChecked(); + await expect( + settingsList.locator('label').first() + ).toHaveText('Allow local nbgrader config file'); + }); + + test('should add option to open formgrader locally', async ({ page, tmpPath }) => { + if (isNotebook) await page.goto(`tree/${tmpPath}`); + + const nbgrader_menu = page.locator( + `${menuPanelId} div.lm-MenuBar-itemLabel:text("Nbgrader")` + ); + const formgrader_menu = page.locator( + '#jp-mainmenu-nbgrader li[data-command="nbgrader:open-formgrader-local"]' + ); + await nbgrader_menu.click(); + expect(formgrader_menu).toHaveCount(0); + + const settings = await openSettings(page); + const formgraderSettings = settings.locator( + '.jp-PluginList-entry[data-id="@jupyter/nbgrader:formgrader"]' + ); + await formgraderSettings.click(); + await settings + .locator('.jp-SettingsPanel fieldset > .form-group input') + .first() + .check(); + + // wait for the settings to be saved + const settingsTab = page + .getByRole('main') + .locator('.lm-TabBar-tab[data-id="setting-editor"]'); + await expect(settingsTab).toHaveAttribute('class', /jp-mod-dirty/); + await expect(settingsTab).not.toHaveAttribute('class', /jp-mod-dirty/); + await nbgrader_menu.click(); + expect(formgrader_menu).toHaveCount(1); + }); + + test('should open formgrader locally', async ({ page, tmpPath }) => { + if (isNotebook) await page.goto(`tree/${tmpPath}`); + + const nbgraderMenu = page.locator( + `${menuPanelId} div.lm-MenuBar-itemLabel:text("Nbgrader")` + ); + const formgraderMenu = page.locator( + '#jp-mainmenu-nbgrader li[data-command="nbgrader:open-formgrader"]' + ); + const localFormgraderMenu = page.locator( + '#jp-mainmenu-nbgrader li[data-command="nbgrader:open-formgrader-local"]' + ); + + const settings = await openSettings(page); + const formgraderSettings = settings.locator( + '.jp-PluginList-entry[data-id="@jupyter/nbgrader:formgrader"]' + ); + await formgraderSettings.click(); + await settings + .locator('.jp-SettingsPanel fieldset > .form-group input') + .first() + .check(); + + // wait for the settings to be saved + const settingsTab = page + .getByRole('main') + .locator('.lm-TabBar-tab[data-id="setting-editor"]'); + await expect(settingsTab).toHaveAttribute('class', /jp-mod-dirty/); + await expect(settingsTab).not.toHaveAttribute('class', /jp-mod-dirty/); + + // Add a local formgrader in another directory + const newDirectory = path.resolve(testDir, 'localFormgrader'); + + if (fs.existsSync(newDirectory)) { + fs.rmSync(newDirectory, { recursive: true}); + } + fs.mkdirSync(newDirectory); + fs.copyFileSync( + path.resolve(testDir, "nbgrader_config.py"), + path.resolve(testDir, tmpPath, "nbgrader_config.py") + ); + + var text_to_append = ` +c.CourseDirectory.course_id = "test_course" +c.Exchange.root = r"${exchange_dir}" +c.Exchange.cache = r"${cache_dir}" +c.Exchange.assignment_dir = r"${newDirectory}" + +`; + + fs.appendFileSync( + path.resolve(newDirectory, "nbgrader_config.py"), + text_to_append + ); + + // open regular formgrader and expect warning because of wrong configuration + await nbgraderMenu.click(); + await formgraderMenu.click(); + let iframe = page.mainFrame().childFrames()[0]; + await (await iframe.frameElement()).contentFrame(); + await expect(iframe.locator('#warning-exchange')).toBeAttached(); + await page.activity.closeAll(); + + // open local formgrader and expect no warning + await page.filebrowser.openDirectory('localFormgrader'); + await nbgraderMenu.click(); + await localFormgraderMenu.click(); + iframe = page.mainFrame().childFrames()[0]; + await (await iframe.frameElement()).contentFrame(); + await expect(iframe.locator('#warning-exchange')).not.toBeAttached(); + }); +}); From a16bc2c0656fcc80353cdb245a45e62339ed0b4c Mon Sep 17 00:00:00 2001 From: Nicolas Brichet Date: Tue, 18 Jun 2024 15:38:43 +0200 Subject: [PATCH 11/12] Fix ui-tests on notebook --- nbgrader/tests/ui-tests/formgrader.spec.ts | 38 ++++++++++++---------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/nbgrader/tests/ui-tests/formgrader.spec.ts b/nbgrader/tests/ui-tests/formgrader.spec.ts index 03a1e239e..fc9548607 100644 --- a/nbgrader/tests/ui-tests/formgrader.spec.ts +++ b/nbgrader/tests/ui-tests/formgrader.spec.ts @@ -100,20 +100,18 @@ test.afterEach(async ({ request, page, tmpPath }) => { await contents.deleteDirectory(tmpPath); }); -const openSettings = async (page: IJupyterLabPageFixture): Promise => { +const openSettings = async (page: IJupyterLabPageFixture): Promise => { await page.evaluate(async () => { await window.jupyterapp.commands.execute('settingeditor:open'); }); // Activate the settings tab, sometimes it does not automatically. - const settingsTab = page - .getByRole('main') - .locator('.lm-TabBar-tab[data-id="setting-editor"]'); + const settingsTab = page.getByRole('tab', { name: 'Settings', exact: true }); await settingsTab.click(); await page.waitForCondition( async () => (await settingsTab.getAttribute('aria-selected')) === 'true' ); - return (await page.activity.getPanelLocator('Settings')) as Locator; + return [(await page.activity.getPanelLocator('Settings')) as Locator, settingsTab]; }; /* @@ -894,7 +892,7 @@ test.describe('#localFormgrader', () => { if (isNotebook) await page.goto(`tree/${tmpPath}`); - const settings = await openSettings(page); + const [settings, settingsTab] = await openSettings(page); const formgraderSettings = settings.locator( '.jp-PluginList-entry[data-id="@jupyter/nbgrader:formgrader"]' ); @@ -914,7 +912,7 @@ test.describe('#localFormgrader', () => { ).toHaveText('Allow local nbgrader config file'); }); - test('should add option to open formgrader locally', async ({ page, tmpPath }) => { + test('should add a menu item to open formgrader locally', async ({ page, tmpPath }) => { if (isNotebook) await page.goto(`tree/${tmpPath}`); const nbgrader_menu = page.locator( @@ -924,9 +922,11 @@ test.describe('#localFormgrader', () => { '#jp-mainmenu-nbgrader li[data-command="nbgrader:open-formgrader-local"]' ); await nbgrader_menu.click(); - expect(formgrader_menu).toHaveCount(0); + await expect(formgrader_menu).not.toBeVisible(); + // close the menu + await nbgrader_menu.click(); - const settings = await openSettings(page); + const [settings, settingsTab] = await openSettings(page); const formgraderSettings = settings.locator( '.jp-PluginList-entry[data-id="@jupyter/nbgrader:formgrader"]' ); @@ -937,9 +937,6 @@ test.describe('#localFormgrader', () => { .check(); // wait for the settings to be saved - const settingsTab = page - .getByRole('main') - .locator('.lm-TabBar-tab[data-id="setting-editor"]'); await expect(settingsTab).toHaveAttribute('class', /jp-mod-dirty/); await expect(settingsTab).not.toHaveAttribute('class', /jp-mod-dirty/); await nbgrader_menu.click(); @@ -959,7 +956,7 @@ test.describe('#localFormgrader', () => { '#jp-mainmenu-nbgrader li[data-command="nbgrader:open-formgrader-local"]' ); - const settings = await openSettings(page); + const [settings, settingsTab] = await openSettings(page); const formgraderSettings = settings.locator( '.jp-PluginList-entry[data-id="@jupyter/nbgrader:formgrader"]' ); @@ -970,9 +967,6 @@ test.describe('#localFormgrader', () => { .check(); // wait for the settings to be saved - const settingsTab = page - .getByRole('main') - .locator('.lm-TabBar-tab[data-id="setting-editor"]'); await expect(settingsTab).toHaveAttribute('class', /jp-mod-dirty/); await expect(settingsTab).not.toHaveAttribute('class', /jp-mod-dirty/); @@ -1007,10 +1001,18 @@ c.Exchange.assignment_dir = r"${newDirectory}" let iframe = page.mainFrame().childFrames()[0]; await (await iframe.frameElement()).contentFrame(); await expect(iframe.locator('#warning-exchange')).toBeAttached(); - await page.activity.closeAll(); + + const formgraderTab = page.getByRole('tab', { name: 'Formgrader', exact: true }); + await formgraderTab.locator('.lm-TabBar-tabCloseIcon').click(); // open local formgrader and expect no warning - await page.filebrowser.openDirectory('localFormgrader'); + if (isNotebook) { + await page.getByRole('tab', { name: 'Files', exact: true }).click(); + await page.locator('.jp-BreadCrumbs-home').click(); + await page.getByText('localFormgrader').last().click({ clickCount: 2}); + } else { + await page.filebrowser.openDirectory('localFormgrader'); + } await nbgraderMenu.click(); await localFormgraderMenu.click(); iframe = page.mainFrame().childFrames()[0]; From d1ba2ff9e90dedc5223969787e433aa375ba4ec3 Mon Sep 17 00:00:00 2001 From: Nicolas Brichet Date: Tue, 18 Jun 2024 16:07:11 +0200 Subject: [PATCH 12/12] Skip test on formgrader exchange on windows --- nbgrader/tests/ui-tests/formgrader.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nbgrader/tests/ui-tests/formgrader.spec.ts b/nbgrader/tests/ui-tests/formgrader.spec.ts index fc9548607..7c4d81cdc 100644 --- a/nbgrader/tests/ui-tests/formgrader.spec.ts +++ b/nbgrader/tests/ui-tests/formgrader.spec.ts @@ -888,7 +888,6 @@ test("Switch views", async ({ page, baseURL, request, tmpPath }) => { */ test.describe('#localFormgrader', () => { test("Should have formgrader settings", async ({ page, tmpPath }) => { - test.skip(isWindows, "This test does not work on Windows"); if (isNotebook) await page.goto(`tree/${tmpPath}`); @@ -944,6 +943,7 @@ test.describe('#localFormgrader', () => { }); test('should open formgrader locally', async ({ page, tmpPath }) => { + test.skip(isWindows, "This test does not work on Windows"); if (isNotebook) await page.goto(`tree/${tmpPath}`); const nbgraderMenu = page.locator(