diff --git a/nbgrader/converters/autograde.py b/nbgrader/converters/autograde.py index b4ceac7ab..1e32ce6d4 100644 --- a/nbgrader/converters/autograde.py +++ b/nbgrader/converters/autograde.py @@ -10,6 +10,7 @@ from ..preprocessors import ( AssignLatePenalties, ClearOutput, DeduplicateIds, OverwriteCells, SaveAutoGrades, Execute, LimitOutput, OverwriteKernelspec, CheckCellMetadata) +from ..postprocessors import CheckDuplicateFlag from ..api import Gradebook, MissingEntry from .. import utils @@ -195,3 +196,6 @@ def convert_single_notebook(self, notebook_filename: str) -> None: super(Autograde, self).convert_single_notebook(notebook_filename) finally: self._sanitizing = True + + self.log.info(f"Post-processing {notebook_filename}") + CheckDuplicateFlag(notebook_filename) diff --git a/nbgrader/converters/base.py b/nbgrader/converters/base.py index 209a6dabf..4b5124e8f 100644 --- a/nbgrader/converters/base.py +++ b/nbgrader/converters/base.py @@ -17,6 +17,7 @@ from ..coursedir import CourseDirectory from ..utils import find_all_files, rmtree, remove from ..preprocessors.execute import UnresponsiveKernelError +from ..postprocessors import DuplicateIdError from ..nbgraderformat import SchemaTooOldError, SchemaTooNewError import typing from nbconvert.exporters.exporter import ResourcesDict @@ -405,6 +406,13 @@ def _handle_failure(gd: typing.Dict[str, str]) -> None: errors.append((gd['assignment_id'], gd['student_id'])) _handle_failure(gd) + except DuplicateIdError: + self.log.error( + f"Encountered a cell with duplicate id when processing {notebook_filename}. " + "Autograding with skipping cells marked as duplicate." + ) + errors.append((gd['assignment_id'], gd['student_id'])) + # Raise unhandled exceptions for the outer try/except except Exception as e: raise e diff --git a/nbgrader/nbgraderformat/__init__.py b/nbgrader/nbgraderformat/__init__.py index 68529f3f4..0a0494682 100644 --- a/nbgrader/nbgraderformat/__init__.py +++ b/nbgrader/nbgraderformat/__init__.py @@ -4,3 +4,9 @@ from .v4 import reads_v4 as reads, writes_v4 as writes SCHEMA_VERSION = MetadataValidator.schema_version + +# Metadata required by latest schema, along with default values +SCHEMA_REQUIRED = {"schema_version": 4, + "grade": False, + "locked": False, + "solution": False} diff --git a/nbgrader/postprocessors/__init__.py b/nbgrader/postprocessors/__init__.py new file mode 100644 index 000000000..8643b2413 --- /dev/null +++ b/nbgrader/postprocessors/__init__.py @@ -0,0 +1,6 @@ +from .checkduplicateflag import CheckDuplicateFlag, DuplicateIdError + +__all__ = [ + "CheckDuplicateFlag", + "DuplicateIdError" +] \ No newline at end of file diff --git a/nbgrader/postprocessors/checkduplicateflag.py b/nbgrader/postprocessors/checkduplicateflag.py new file mode 100644 index 000000000..b5acac955 --- /dev/null +++ b/nbgrader/postprocessors/checkduplicateflag.py @@ -0,0 +1,27 @@ +import nbformat +from nbformat.notebooknode import NotebookNode + + +class DuplicateIdError(Exception): + + def __init__(self, message): + super(DuplicateIdError, self).__init__(message) + + +class CheckDuplicateFlag: + + def __init__(self, notebook_filename): + with open(notebook_filename, encoding="utf-8") as f: + nb = nbformat.read(f, as_version=nbformat.NO_CONVERT) + self.postprocess(nb) + + def postprocess(self, nb: NotebookNode): + for cell in nb.cells: + self.postprocess_cell(cell) + + @staticmethod + def postprocess_cell(cell: NotebookNode): + if "nbgrader" in cell.metadata and "duplicate" in cell.metadata.nbgrader: + del cell.metadata.nbgrader["duplicate"] + msg = "Detected cells with same ids" + raise DuplicateIdError(msg) diff --git a/nbgrader/preprocessors/deduplicateids.py b/nbgrader/preprocessors/deduplicateids.py index 9251ab61e..916057560 100644 --- a/nbgrader/preprocessors/deduplicateids.py +++ b/nbgrader/preprocessors/deduplicateids.py @@ -1,5 +1,6 @@ from .. import utils from . import NbGraderPreprocessor +from ..nbgraderformat import SCHEMA_REQUIRED from nbconvert.exporters.exporter import ResourcesDict from nbformat.notebooknode import NotebookNode from typing import Tuple @@ -27,7 +28,9 @@ def preprocess_cell(self, grade_id = cell.metadata.nbgrader['grade_id'] if grade_id in self.grade_ids: self.log.warning("Cell with id '%s' exists multiple times!", grade_id) - cell.metadata.nbgrader = {} + # Replace problematic metadata and leave message + cell.source = "# THIS CELL CONTAINED A DUPLICATE ID DURING AUTOGRADING\n" + cell.source + cell.metadata.nbgrader = SCHEMA_REQUIRED | {"duplicate": True} else: self.grade_ids.add(grade_id) diff --git a/nbgrader/tests/preprocessors/test_deduplicateids.py b/nbgrader/tests/preprocessors/test_deduplicateids.py index e3f513b04..9a319ec6a 100644 --- a/nbgrader/tests/preprocessors/test_deduplicateids.py +++ b/nbgrader/tests/preprocessors/test_deduplicateids.py @@ -3,6 +3,7 @@ from nbformat.v4 import new_notebook from ...preprocessors import DeduplicateIds +from ...nbgraderformat import SCHEMA_REQUIRED from .base import BaseTestPreprocessor from .. import ( create_grade_cell, create_solution_cell, create_locked_cell) @@ -14,6 +15,10 @@ def preprocessor(): return pp +EXPECTED_DUPLICATE_METADATA = SCHEMA_REQUIRED # | {"duplicate": True} # doesn't work in python 3.8 +EXPECTED_DUPLICATE_METADATA["duplicate"] = True + + class TestDeduplicateIds(BaseTestPreprocessor): def test_duplicate_grade_cell(self, preprocessor): @@ -26,7 +31,7 @@ def test_duplicate_grade_cell(self, preprocessor): nb, resources = preprocessor.preprocess(nb, {}) assert nb.cells[0].metadata.nbgrader != {} - assert nb.cells[1].metadata.nbgrader == {} + assert nb.cells[1].metadata.nbgrader == EXPECTED_DUPLICATE_METADATA def test_duplicate_solution_cell(self, preprocessor): cell1 = create_solution_cell("hello", "code", "foo") @@ -38,7 +43,7 @@ def test_duplicate_solution_cell(self, preprocessor): nb, resources = preprocessor.preprocess(nb, {}) assert nb.cells[0].metadata.nbgrader != {} - assert nb.cells[1].metadata.nbgrader == {} + assert nb.cells[1].metadata.nbgrader == EXPECTED_DUPLICATE_METADATA def test_duplicate_locked_cell(self, preprocessor): cell1 = create_locked_cell("hello", "code", "foo") @@ -50,4 +55,4 @@ def test_duplicate_locked_cell(self, preprocessor): nb, resources = preprocessor.preprocess(nb, {}) assert nb.cells[0].metadata.nbgrader != {} - assert nb.cells[1].metadata.nbgrader == {} + assert nb.cells[1].metadata.nbgrader == EXPECTED_DUPLICATE_METADATA