Skip to content

Commit

Permalink
add student_files to otter assign (#852)
Browse files Browse the repository at this point in the history
  • Loading branch information
chrispyles authored Oct 4, 2024
1 parent ac9c9fe commit e03275b
Show file tree
Hide file tree
Showing 16 changed files with 91 additions and 18 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
* Updated Otter Assign to strip type annotations from generated test code per [#796](https://github.com/ucbds-infra/otter-grader/issues/796)
* Updated Otter Grade Docker image to create an empty `submission_metadata.json` file in the grading image to prevent plugins from erroring per [#811](https://github.com/ucbds-infra/otter-grader/issues/811)
* Added ability to monitor grading progress to Otter Grade per [#827](https://github.com/ucbds-infra/otter-grader/issues/827)
* Added handling of student-created files with the `student_files` configuration in Otter Assign per [#737](https://github.com/ucbds-infra/otter-grader/issues/737)

**v5.6.0:**

Expand Down
11 changes: 10 additions & 1 deletion otter/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import os

from contextlib import nullcontext, redirect_stdout
from typing import Optional

from .export import export_notebook
from .run import main as run_grader
Expand All @@ -17,6 +18,7 @@ def grade_submission(
*,
quiet: bool = False,
debug: bool = False,
extra_submission_files: Optional[list[str]] = None,
) -> GradingResults:
"""
Runs non-containerized grading on a single submission at ``submission_path`` using the autograder
Expand All @@ -36,6 +38,8 @@ def grade_submission(
``False``
debug (``bool``): whether to run the submission in debug mode (without ignoring
errors)
extra_submission_files (``list[str] | None``): extra files to copy into the submission
directory; this should really only be used internally by Otter, so use at your own risk
Returns:
``otter.test_files.GradingResults``: the results object produced during the grading of the
Expand All @@ -49,7 +53,12 @@ def grade_submission(

with cm:
results = run_grader(
submission_path, autograder=ag_path, output_dir=None, no_logo=True, debug=debug
submission_path,
autograder=ag_path,
output_dir=None,
no_logo=True,
debug=debug,
extra_submission_files=extra_submission_files,
)

if quiet:
Expand Down
16 changes: 11 additions & 5 deletions otter/assign/assignment.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ class ExportCellValue(fica.Config):

files: list[str] = fica.Key(
description="a list of other files to include in the student submissions' zip file",
default=[],
factory=lambda: [],
)

class RequireNoPDFAckValue(fica.Config):
Expand Down Expand Up @@ -157,22 +157,28 @@ class SeedValue(fica.Config):

ignore_modules: list[str] = fica.Key(
description="a list of modules to ignore variables from during environment serialization",
default=[],
factory=lambda: [],
)

files: list[str] = fica.Key(
description="a list of other files to include in the output directories and autograder",
default=[],
factory=lambda: [],
)

autograder_files: list[str] = fica.Key(
description="a list of other files only to include in the autograder",
default=[],
factory=lambda: [],
)

student_files: list[str] = fica.Key(
description="a list of files that will be generated by students and thus should not be included in the autograder configuration zip file",
factory=lambda: [],
type_=list,
)

plugins: list[str] = fica.Key(
description="a list of plugin names and configurations",
default=[],
factory=lambda: [],
)

class TestsValue(fica.Config):
Expand Down
5 changes: 3 additions & 2 deletions otter/assign/cell_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,9 @@ def create_export_cells(self) -> list[nbformat.NotebookNode]:
args += ["force_save=True"]
if self.assignment.export_cell.run_tests:
args += ["run_tests=True"]
if len(self.assignment.export_cell.files) != 0:
args += [f"files={self.assignment.export_cell.files}"]
if self.assignment.export_cell.files or self.assignment.student_files:
l = [*self.assignment.export_cell.files, *self.assignment.student_files]
args += [f"files={l}"]
source_lines.append(f"grader.export({', '.join(args)})")
export.source = "\n".join(source_lines)

Expand Down
15 changes: 15 additions & 0 deletions otter/assign/output.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,21 @@ def write_output_dir(
output_fn = "environment.yml"
shutil.copy(assignment.environment, str(output_dir / output_fn))

if assignment.student_files:
for file in assignment.student_files:
# TODO: dedupe this logic with handling of autograder_files utils.py and logic below
# for assignment.files
if not os.path.isfile(file):
raise FileNotFoundError(f"{file} is not a file")
if str(assignment.master.parent) not in os.path.abspath(file):
raise ValueError(
f"{file} is not in a subdirectory of the master notebook direcotry"
)
file_path = pathlib.Path(file).resolve()
rel_path = file_path.parent.relative_to(assignment.master.parent)
os.makedirs(output_dir / rel_path, exist_ok=True)
shutil.copy(file, str(output_dir / rel_path))

# write tests
transformed_nb.write_tests(str(tests_dir), not sanitize, assignment.tests.files)

Expand Down
3 changes: 3 additions & 0 deletions otter/assign/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
import pathlib
import re
import shutil
import tempfile
import zipfile

from textwrap import indent
from typing import Any, Optional, TYPE_CHECKING
Expand Down Expand Up @@ -224,6 +226,7 @@ def run_tests(assignment: "Assignment", debug: bool = False) -> None:
str(assignment.ag_notebook_path),
str(assignment.ag_zip_path),
debug=debug,
extra_submission_files=assignment.student_files,
)

LOGGER.debug(f"Otter Run output:\n{run_output.getvalue()}")
Expand Down
12 changes: 12 additions & 0 deletions otter/run/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,13 @@
import dill
import json
import os
import pathlib
import shutil
import tempfile
import zipfile

from typing import Optional

from .run_autograder import AutograderConfig, capture_run_output, main as run_autograder_main
from ..test_files import GradingResults

Expand All @@ -21,6 +24,7 @@ def main(
output_dir: str = "./",
no_logo: bool = False,
debug: bool = False,
extra_submission_files: Optional[list[str]] = None,
) -> GradingResults:
"""
Grades a single submission using the autograder configuration ``autograder`` without
Expand All @@ -37,6 +41,8 @@ def main(
the results JSON file is not copied
no_logo (``bool``): whether to suppress the Otter logo from being printed to stdout
debug (``bool``); whether to run in debug mode (without ignoring errors)
extra_submission_files (``list[str] | None``): extra files to copy into the submission
directory; this should really only be used internally by Otter, so use at your own risk
Returns:
``otter.test_files.GradingResults``: the grading results object
Expand Down Expand Up @@ -65,6 +71,12 @@ def main(
else:
shutil.copy(submission, os.path.join(ag_dir, "submission"))

for file in extra_submission_files:
fp = pathlib.Path(os.path.join(ag_dir, file))
# create any intermediate directories before copying the file
fp.parents[0].mkdir(parents=True, exist_ok=True)
shutil.copy(file, os.path.join(ag_dir, "submission", file))

logo = not no_logo
run_autograder_main(ag_dir, logo=logo, debug=debug, otter_run=True)

Expand Down
1 change: 1 addition & 0 deletions test/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ def test_grade_submission(mocked_redirect, mocked_run):
output_dir=None,
no_logo=True,
debug=False,
extra_submission_files=None,
)
mocked_redirect.assert_not_called()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@
'locked': False,
'points': 10,
'success_message': 'Congrats you passed this test case!'},
{'code': ">>> str(print(x))\n2\n'None'", 'hidden': True, 'locked': False, 'success_message': 'Congrats you passed this test case!'}],
{'code': ">>> str(print(x))\n2\n'None'", 'hidden': True, 'locked': False, 'success_message': 'Congrats you passed this test case!'},
{'code': '>>> import student_file\n>>> assert student_file.FOO == 1\n', 'hidden': False, 'locked': False}],
'scored': True,
'setup': '',
'teardown': '',
Expand Down
Binary file not shown.
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,7 @@
},
"outputs": [],
"source": [
"grader.export(force_save=True, run_tests=True)"
"grader.export(force_save=True, run_tests=True, files=['student_file.py'])"
]
},
{
Expand All @@ -542,7 +542,7 @@
"name": "python",
"nbconvert_exporter": "python",
"pygments_lexer": "ipython3",
"version": "3.11.4"
"version": "3.9.19"
},
"otter": {
"OK_FORMAT": true,
Expand Down Expand Up @@ -590,6 +590,11 @@
"hidden": true,
"locked": false,
"success_message": "Congrats you passed this test case!"
},
{
"code": ">>> import student_file\n>>> assert student_file.FOO == 1\n",
"hidden": false,
"locked": false
}
],
"scored": true,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
FOO = 1
9 changes: 7 additions & 2 deletions test/test_assign/files/example-correct/student/example.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@
},
"outputs": [],
"source": [
"grader.export(force_save=True, run_tests=True)"
"grader.export(force_save=True, run_tests=True, files=['student_file.py'])"
]
},
{
Expand All @@ -433,7 +433,7 @@
"name": "python",
"nbconvert_exporter": "python",
"pygments_lexer": "ipython3",
"version": "3.11.4"
"version": "3.9.19"
},
"otter": {
"OK_FORMAT": true,
Expand Down Expand Up @@ -467,6 +467,11 @@
"locked": false,
"points": 1,
"success_message": "Congrats your x value is in the correct range!"
},
{
"code": ">>> import student_file\n>>> assert student_file.FOO == 1\n",
"hidden": false,
"locked": false
}
],
"scored": true,
Expand Down
20 changes: 15 additions & 5 deletions test/test_assign/files/example.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@
"python_version: 3.8\n",
"exclude_conda_defaults: true\n",
"files:\n",
" - data.csv"
" - data.csv\n",
"student_files:\n",
" - student_file.py"
]
},
{
Expand Down Expand Up @@ -176,9 +178,7 @@
{
"cell_type": "code",
"execution_count": 7,
"metadata": {
"scrolled": true
},
"metadata": {},
"outputs": [
{
"name": "stdout",
Expand Down Expand Up @@ -207,6 +207,16 @@
"str(print(x))"
]
},
{
"cell_type": "code",
"execution_count": 1,
"metadata": {},
"outputs": [],
"source": [
"import student_file\n",
"assert student_file.FOO == 1"
]
},
{
"cell_type": "raw",
"metadata": {},
Expand Down Expand Up @@ -971,7 +981,7 @@
"name": "python",
"nbconvert_exporter": "python",
"pygments_lexer": "ipython3",
"version": "3.11.4"
"version": "3.9.19"
},
"varInspector": {
"cols": {
Expand Down
1 change: 1 addition & 0 deletions test/test_assign/files/student_file.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
FOO = 1
2 changes: 2 additions & 0 deletions test/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,8 @@ def test_run(mocked_run, run_cli):
submission="foo.ipynb",
**run.__kwdefaults__,
)
# extra_submission_files is not used by the CLI
std_kwargs.pop("extra_submission_files")

result = run_cli([*cmd_start])
assert_cli_result(result, expect_error=False)
Expand Down

0 comments on commit e03275b

Please sign in to comment.