Skip to content

Commit

Permalink
Merge pull request #1179 from AaltoScienceIT/exclude_student_id
Browse files Browse the repository at this point in the history
Add CourseDir.student_id_exclude option to exclude students
  • Loading branch information
jhamrick authored Aug 24, 2019
2 parents 0fae03e + 872067f commit 328e41f
Show file tree
Hide file tree
Showing 6 changed files with 103 additions and 3 deletions.
5 changes: 5 additions & 0 deletions nbgrader/converters/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,11 @@ def init_destination(self, assignment_id, student_id):
initialization was successful).
"""
if self.coursedir.student_id_exclude:
exclude_ids = self.coursedir.student_id_exclude.split(',')
if student_id in exclude_ids:
return False

dest = os.path.normpath(self._format_dest(assignment_id, student_id))

# the destination doesn't exist, so we haven't processed it
Expand Down
13 changes: 13 additions & 0 deletions nbgrader/coursedir.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,19 @@ def _validate_student_id(self, proposal):
self.log.warning("student_id '%s' has trailing whitespace, stripping it away", proposal['value'])
return proposal['value'].strip()

student_id_exclude = Unicode(
"",
help=dedent(
"""
Comma-separated list of student IDs to exclude. Counterpart of student_id.
This is useful when running commands on all students, but certain
students cause errors or otherwise must be left out. Works at
least for autograde, generate_feedback, and release_feedback.
"""
)
).tag(config=True)

assignment_id = Unicode(
"",
help=dedent(
Expand Down
10 changes: 8 additions & 2 deletions nbgrader/exchange/release_feedback.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,18 @@ def copy_files(self):
self.log.info("student_id: {}".format(student_id))
html_files = glob.glob(os.path.join(self.src_path, student_id, self.coursedir.assignment_id, '*.html'))
self.log.info("html_files: {}".format(html_files))
if self.coursedir.student_id_exclude:
exclude_students = set(self.coursedir.student_id_exclude.split(','))
else:
exclude_students = set()
for html_file in html_files:
assignment_dir, file_name = os.path.split(html_file)
timestamp = open(os.path.join(assignment_dir, 'timestamp.txt')).read()
self.log.info("timestamp {}".format(timestamp))
user = assignment_dir.split('/')[-2]
submissionDir = os.path.join(self.src_path, '../submitted/', '{0}/{1}'.format(user, self.coursedir.assignment_id))
student_id = assignment_dir.split('/')[-2]
if student_id in exclude_students:
continue
submissionDir = os.path.join(self.src_path, '../submitted/', '{0}/{1}'.format(student_id, self.coursedir.assignment_id))
fname, _ = os.path.splitext(file_name.replace('.html', ''))
self.log.info("found html file {}".format(fname))
nbfile = "{0}/{1}.ipynb".format(submissionDir, fname)
Expand Down
30 changes: 29 additions & 1 deletion nbgrader/tests/apps/test_nbgrader_autograde.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from textwrap import dedent
from nbformat import current_nbformat

from ...api import Gradebook
from ...api import Gradebook, MissingEntry
from ...utils import remove
from ...nbgraderformat import reads
from .. import run_nbgrader
Expand Down Expand Up @@ -94,6 +94,34 @@ def test_grade(self, db, course_dir):
assert comment1.comment == None
assert comment2.comment == None

def test_student_id_exclude(self, db, course_dir):
"""Does --CourseDirectory.student_id_exclude=X exclude students?"""
with open("nbgrader_config.py", "a") as fh:
fh.write("""c.CourseDirectory.db_assignments = [dict(name='ps1', duedate='2015-02-02 14:58:23.948203 America/Los_Angeles')]\n""")
fh.write("""c.CourseDirectory.db_students = [dict(id="foo"), dict(id="bar"), dict(id="baz")]""")

self._copy_file(join("files", "submitted-unchanged.ipynb"), join(course_dir, "source", "ps1", "p1.ipynb"))
run_nbgrader(["generate_assignment", "ps1", "--db", db])

self._copy_file(join("files", "submitted-unchanged.ipynb"), join(course_dir, "submitted", "foo", "ps1", "p1.ipynb"))
self._copy_file(join("files", "submitted-changed.ipynb"), join(course_dir, "submitted", "bar", "ps1", "p1.ipynb"))
self._copy_file(join("files", "submitted-changed.ipynb"), join(course_dir, "submitted", "baz", "ps1", "p1.ipynb"))
run_nbgrader(["autograde", "ps1", "--db", db, '--CourseDirectory.student_id_exclude=bar,baz'])

assert os.path.isfile(join(course_dir, "autograded", "foo", "ps1", "p1.ipynb"))
assert not os.path.isfile(join(course_dir, "autograded", "bar", "ps1", "p1.ipynb"))
assert not os.path.isfile(join(course_dir, "autograded", "baz", "ps1", "p1.ipynb"))

with Gradebook(db) as gb:
notebook = gb.find_submission_notebook("p1", "ps1", "foo")
assert notebook.score == 1

with pytest.raises(MissingEntry):
notebook = gb.find_submission_notebook("p1", "ps1", "bar")
with pytest.raises(MissingEntry):
notebook = gb.find_submission_notebook("p1", "ps1", "baz")


def test_grade_timestamp(self, db, course_dir):
"""Is a timestamp correctly read in?"""
with open("nbgrader_config.py", "a") as fh:
Expand Down
21 changes: 21 additions & 0 deletions nbgrader/tests/apps/test_nbgrader_generate_feedback.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,27 @@ def test_single_file(self, db, course_dir):

assert exists(join(course_dir, "feedback", "foo", "ps1", "p1.html"))

def test_student_id_exclude(self, db, course_dir):
"""Does --CourseDirectory.student_id_exclude=X exclude students?"""
with open("nbgrader_config.py", "a") as fh:
fh.write("""c.CourseDirectory.db_assignments = [dict(name="ps1")]\n""")
fh.write("""c.CourseDirectory.db_students = [dict(id="foo"), dict(id="bar"), dict(id="baz")]\n""")
self._copy_file(join("files", "submitted-unchanged.ipynb"), join(course_dir, "source", "ps1", "p1.ipynb"))
run_nbgrader(["assign", "ps1", "--db", db])

for student in ["foo", "bar", "baz"]:
self._copy_file(join("files", "submitted-unchanged.ipynb"), join(course_dir, "submitted", student, "ps1", "p1.ipynb"))
run_nbgrader(["autograde", "ps1", "--db", db])
run_nbgrader(["generate_feedback", "ps1", "--db", db, "--CourseDirectory.student_id_exclude=bar,baz"])

for student in ["foo", "bar", "baz"]:
assert exists(join(course_dir, "autograded", "foo", "ps1", "p1.ipynb"))

assert exists(join(course_dir, "feedback", "foo", "ps1", "p1.html"))
assert not exists(join(course_dir, "feedback", "bar", "ps1", "p1.html"))
assert not exists(join(course_dir, "feedback", "baz", "ps1", "p1.html"))


def test_force(self, db, course_dir):
"""Ensure the force option works properly"""
with open("nbgrader_config.py", "a") as fh:
Expand Down
27 changes: 27 additions & 0 deletions nbgrader/tests/apps/test_nbgrader_releasefeedback.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,33 @@ def test_single_student(self, db, course_dir, exchange):
# release feedback shoul overwrite without error
run_nbgrader(["release_feedback", "ps1", "--Exchange.root={}".format(exchange), '--course', 'abc101'])

@notwindows
def test_student_id_exclude(self, db, course_dir, exchange):
"""Does --CourseDirectory.student_id_exclude=X exclude students?"""
with open("nbgrader_config.py", "a") as fh:
fh.write("""c.CourseDirectory.db_assignments = [dict(name="ps1")]\n""")
fh.write("""c.CourseDirectory.db_students = [dict(id="foo"), dict(id="bar"), dict(id="baz")]\n""")
self._copy_file(join("files", "submitted-unchanged.ipynb"), join(course_dir, "source", "ps1", "p1.ipynb"))
run_nbgrader(["assign", "ps1", "--db", db])
nb_path = join(course_dir, "submitted", "foo", "ps1", "p1.ipynb")
self._copy_file(join("files", "submitted-unchanged.ipynb"), nb_path)
self._copy_file(join("files", "timestamp.txt"), join(course_dir, "submitted", "foo", "ps1", "timestamp.txt"))
nb_path2 = join(course_dir, "submitted", "bar", "ps1", "p1.ipynb")
self._copy_file(join("files", "submitted-changed.ipynb"), nb_path2)
self._copy_file(join("files", "timestamp.txt"), join(course_dir, "submitted", "bar", "ps1", "timestamp.txt"))

run_nbgrader(["autograde", "ps1", "--db", db])
run_nbgrader(["generate_feedback", "ps1", "--db", db])
run_nbgrader(["release_feedback", "ps1", "--Exchange.root={}".format(exchange), '--course', 'abc101',
"--CourseDirectory.student_id_exclude=bar,baz"]) # baz doesn't exist, test still OK though
nb_hash = notebook_hash(nb_path) # foo
assert exists(join(exchange, "abc101", "feedback", "{}.html".format(nb_hash)))
nb_hash2 = notebook_hash(nb_path2) # bar
assert not exists(join(exchange, "abc101", "feedback", "{}.html".format(nb_hash2)))
# release feedback should overwrite without error
run_nbgrader(["release_feedback", "ps1", "--Exchange.root={}".format(exchange), '--course', 'abc101'])


@notwindows
@pytest.mark.parametrize("groupshared", [False, True])
def test_permissions(self, db, course_dir, exchange, groupshared):
Expand Down

0 comments on commit 328e41f

Please sign in to comment.