Merge pull request #741 from ucbds-infra/beta

.github/workflows/run-tests.yml

@@ -35,6 +35,10 @@ jobs:
         with:
           install: true
 
+      - uses: r-lib/actions/setup-tinytex@v2
+
+      - uses: r-lib/actions/setup-pandoc@v2
+
       - uses: mamba-org/setup-micromamba@v1
         with:
           environment-file: environment.yml

.gitignore

@@ -22,3 +22,4 @@ docs/_static/notebooks/html
 htmlcov/
 .pytest_cache/
 /sync.sh
+pytest-report.html

CHANGELOG.md

@@ -1,5 +1,12 @@
 # Changelog
 
+**v5.2.0 (unreleased):**
+
+* Migrate installation of `ottr` from `setup.sh` to `environment.yml` with the [`r-ottr` conda-forge recipe](https://anaconda.org/conda-forge/r-ottr)
+* Updated Otter Assign to allow multiline statements in test cases for Python 3.9+ per [#590](https://github.com/ucbds-infra/otter-grader/issues/590)
+* Added `otter_include` tag to allow inclusion of tagged markdown cells within the solution block into the student notebook per [#730](https://github.com/ucbds-infra/otter-grader/issues/730)
+* Removed dependency on `nbconvert` during import so that Otter can be imported on Jupyterlite per [#736](https://github.com/ucbds-infra/otter-grader/issues/736)
+
 **v5.1.4:**
 
 * Prevented the `Notebook` class from attempting to resolve the notebook path when in grading mode

Makefile

@@ -5,7 +5,7 @@ COVERAGE         = coverage
 DOCKER           = true
 SLOW             = true
 
-_PYTESTOPTS      := -vv --durations=0
+_PYTESTOPTS      := -vv --durations=0 --html=pytest-report.html --self-contained-html
 
 ifeq ($(DOCKER), false)
 	_PYTESTOPTS := $(_PYTESTOPTS) -m "not docker"

docs/otter_check/index.rst

@@ -62,12 +62,11 @@ variable name collisions, propagating errors, or other things that would cause t
 fail a test they should be passing.
 
 
-.. TODO: fix this section
-
 Exporting Submissions
 +++++++++++++++++++++
 
-Students can also use the ``Notebook`` class to generate their own PDFs for manual grading using the 
+Students can also use the ``Notebook`` class to generate a zip file containing all of their work for
+submission with the 
 method ``Notebook.export``. This function takes an optional argument of the path to the notebook; if 
 unspecified, it will infer the path by trying to read the config file (if present), using the path 
 of the only notebook in the working directory if there is only one, or it will raise an error 

otter/api.py

@@ -3,8 +3,6 @@
 __all__ = ["export_notebook", "grade_submission"]
 
 import os
-import shutil
-import tempfile
 
 from contextlib import redirect_stdout
 
@@ -41,22 +39,17 @@ def grade_submission(submission_path, ag_path="autograder.zip", quiet=False, deb
         ``otter.test_files.GradingResults``: the results object produced during the grading of the
             submission.
     """
-    dp = tempfile.mkdtemp()
-
     if quiet:
         f = open(os.devnull, "w")
         cm = redirect_stdout(f)
     else:
         cm = nullcontext()
 
-    # TODO: is the output_dir argument of run_grader necessary here?
     with cm:
         results = run_grader(
-            submission_path, autograder=ag_path, output_dir=dp, no_logo=True, debug=debug)
+            submission_path, autograder=ag_path, output_dir=None, no_logo=True, debug=debug)
 
     if quiet:
         f.close()
 
-    shutil.rmtree(dp)
-
     return results

otter/assign/__init__.py

@@ -35,15 +35,15 @@ def main(
         no_pdfs (``bool``): whether to ignore any configurations indicating PDF generation for this run
         no_run_tests (``bool``): prevents Otter tests from being automatically run on the solutions 
             notebook
-        username (``str``): a username for Gradescope for generating a token
-        password (``str``): a password for Gradescope for generating a token
+        username (``str | None``): a username for Gradescope for generating a token
+        password (``str | None``): a password for Gradescope for generating a token
         debug (``bool``): whether to run in debug mode (without ignoring errors during testing)
     """
     LOGGER.debug(f"User-specified master path: {master}")
     LOGGER.debug(f"User-specified result path: {result}")
     master, result = pathlib.Path(os.path.abspath(master)), pathlib.Path(os.path.abspath(result))
 
-    assignment = Assignment()
+    assignment = Assignment(require_valid_keys=True)
 
     result = get_relpath(master.parent, result)
 

otter/assign/assignment.py

@@ -13,7 +13,6 @@
 from ..utils import Loggable
 
 
-# TODO: add detection/warnings/errors for when a user provides an invalid key? (to be added to fica)
 class Assignment(fica.Config, Loggable):
     """
     Configurations for the assignment.
@@ -241,8 +240,7 @@ class TestsValue(fica.Config):
     seed_required: bool = False
     """whether a seeding configuration is required for Otter Generate"""
 
-    # TODO: rename this
-    _temp_test_dir: Optional[str] = None
+    generate_tests_dir: Optional[str] = None
     """the path to a directory of test files for Otter Generate"""
 
     notebook_basename: Optional[str] = None
@@ -267,7 +265,7 @@ def __init__(self, user_config: Dict[str, Any] = {}, **kwargs) -> None:
         if self.variables:
             warnings.warn(
                 "The variables key of the assignment config is deprecated and will be removed in " \
-                    "v6.0.0. Please use generate.seed_variables instead.", DeprecationWarning)
+                    "v6.0.0. Please use generate.serialized_variables instead.", DeprecationWarning)
 
     def update(self, user_config: Dict[str, Any]):
         self._logger.debug(f"Updating config: {user_config}")
@@ -309,7 +307,6 @@ def get_otter_config(self):
         if self.is_r:
             self.generate.lang = "r"
 
-        # TODO: move this config out of the assignment metadata and into the generate key
         if self.variables:
             self.generate.serialized_variables = str(self.variables)
 

otter/assign/feature_toggle.py

@@ -11,7 +11,6 @@
 """a function that returns ``True`` if the assignment is not an R Markdown assignment"""
 
 
-# TODO: move other things in
 class FeatureToggle(Enum):
     """
     An enum for managing features that are enabled or disabled depending on the assignment config.

otter/assign/notebook_transformer.py

@@ -1,6 +1,7 @@
 """Master notebook transformer for Otter Assign"""
 
 import copy
+import fica
 import nbformat
 
 from .assignment import Assignment
@@ -21,8 +22,6 @@
 from .tests_manager import AssignmentTestsManager
 from .utils import (
     add_tag,
-    add_assignment_name_to_notebook,
-    add_require_no_pdf_ack_to_notebook,
     AssignNotebookFormatException,
     get_source,
     is_cell_type,
@@ -31,6 +30,9 @@
     remove_cell_ids_if_applicable,
 )
 
+from ..nbmeta_config import NBMetadataConfig
+from ..utils import NOTEBOOK_METADATA_KEY
+
 
 class NotebookTransformer:
     """
@@ -134,10 +136,6 @@ def transform_notebook(self, nb) -> "TransformedNotebookContainer":
         # strip out ignored lines
         transformed_nb = strip_ignored_lines(transformed_nb)
 
-        add_assignment_name_to_notebook(transformed_nb, self.assignment)
-
-        add_require_no_pdf_ack_to_notebook(transformed_nb, self.assignment)
-
         remove_cell_ids_if_applicable(transformed_nb)
 
         return TransformedNotebookContainer(transformed_nb, self)
@@ -205,7 +203,7 @@ def _get_transformed_cells(self, cells):
                     solution_has_md_cells, prompt_insertion_index = False, None
 
                 elif block_type is BlockType.SOLUTION:
-                    if not has_prompt and solution_has_md_cells:
+                    if not has_prompt and solution_has_md_cells and question.manual:
                         if prompt_insertion_index is None:
                             raise RuntimeError("Could not find prompt insertion index")
                         transformed_cells.insert(
@@ -353,9 +351,26 @@ class TransformedNotebookContainer:
     nb_transformer: NotebookTransformer
     """the notebook transformer used to create ``transformed_nb``"""
 
+    nbmeta_config: NBMetadataConfig
+    """the notebook metadata config to include when writing the notebook"""
+
     def __init__(self, transformed_nb, nb_transformer):
         self.transformed_nb = transformed_nb
         self.nb_transformer = nb_transformer
+        self._populate_nbmeta_config(self.nb_transformer.assignment)
+
+    def _populate_nbmeta_config(self, a: Assignment):
+        """
+        Copy configurations from the ``Assignment`` into the ``NBMetadataConfig``.
+        """
+        self.nbmeta_config = NBMetadataConfig({})
+        if a.name:
+            self.nbmeta_config.assignment_name = a.name
+        if a.export_cell and a.export_cell.require_no_pdf_ack:
+            self.nbmeta_config.require_no_pdf_confirmation = True
+            if isinstance(a.export_cell.require_no_pdf_ack, fica.Config):
+                self.nbmeta_config.export_pdf_failure_message = \
+                    a.export_cell.require_no_pdf_ack.message
 
     def _get_sanitized_nb(self):
         """
@@ -374,6 +389,14 @@ def _get_sanitized_nb(self):
             )
         return nb
 
+    def _add_nbmeta_config(self, nb):
+        """
+        Add the notebook metadata config to the provided notebook's metadata in-place.
+        """
+        uc = self.nbmeta_config.get_user_config()
+        if uc:
+            nb["metadata"][NOTEBOOK_METADATA_KEY] = uc
+
     def write_transformed_nb(self, output_path, sanitize):
         """
         Write the transformed notebook (either as an autograder or student notebook) to the
@@ -384,8 +407,11 @@ def write_transformed_nb(self, output_path, sanitize):
             sanitize (``bool``): whether to sanitize the notebook (i.e. write the student version)
         """
         nb = self._get_sanitized_nb() if sanitize else self.transformed_nb
+        self._add_nbmeta_config(nb)
+
         if self.nb_transformer.assignment.is_rmd:
             rmarkdown_converter.write_as_rmd(nb, str(output_path), not sanitize)
+
         else:
             try:
                 from nbformat.validator import normalize
@@ -407,7 +433,7 @@ def write_tests(self, tests_dir, include_hidden, force_files):
                 assignment config)
         """
         self.nb_transformer.tests_mgr.write_tests(
-            self.transformed_nb,
+            self.nbmeta_config,
             tests_dir,
             include_hidden=include_hidden,
             force_files=force_files,

otter/assign/output.py

@@ -61,8 +61,8 @@ def write_output_dir(
 
     # write a temp dir for otter generate tests
     if not sanitize:
-        assignment._temp_test_dir = pathlib.Path(tempfile.mkdtemp())
-        transformed_nb.write_tests(str(assignment._temp_test_dir), True, True)
+        assignment.generate_tests_dir = tempfile.mkdtemp()
+        transformed_nb.write_tests(assignment.generate_tests_dir, True, True)
 
     transformed_nb.write_transformed_nb(output_path, sanitize)
 
@@ -84,7 +84,7 @@ def write_output_dir(
             shutil.copy(file, str(output_dir / rel_path))
 
 
-def write_output_directories(assignment):
+def write_output_directories(assignment: Assignment):
     """
     Process a master notebook and write the results to the output directories.
 

otter/assign/solutions.py

@@ -195,6 +195,7 @@ def strip_ignored_lines(nb):
         cell['source'] = '\n'.join(remove_ignored_lines(get_source(cell)))
     return nb
 
+OTTER_INCLUDE_TAG = "otter_include"
 
 def strip_solutions_and_output(nb):
     """
@@ -208,21 +209,24 @@ def strip_solutions_and_output(nb):
     """
     nb = copy.deepcopy(nb)
 
-    md_solutions = []
+    del_md_solutions = []
     lang = get_notebook_language(nb)
     for i, cell in enumerate(nb['cells']):
         if has_tag(cell, SOLUTION_CELL_TAG):
             if is_cell_type(cell, "code"):
                 cell['source'] = '\n'.join(replace_solutions(get_source(cell), lang))
             elif is_cell_type(cell, "markdown"):
-                md_solutions.append(i)
+                if has_tag(cell, OTTER_INCLUDE_TAG):
+                    cell = remove_tag(cell, OTTER_INCLUDE_TAG)
+                else:
+                    del_md_solutions.append(i)
             nb['cells'][i] = remove_tag(cell, SOLUTION_CELL_TAG)
 
-    md_solutions.reverse()
-    for i in md_solutions:
+    del_md_solutions.reverse()
+    for i in del_md_solutions:
         del nb['cells'][i]
 
     # remove output from student version
     remove_output(nb)
 
-    return nb
+    return nb