Skip to content

Commit 571888e

Browse files
committed
refactor: reduce reliance on regex in path handling
This change tries to modernize the way we handle paths in nbgrader by using pathlib.Path and centralizing logic for searching and extracting information from paths into CourseDirectory. By using `Path(root).glob(pat)` instead of `glob.glob(root / pat)`, we can avoid some issues of needing to escape CourseDirectory.root because the glob will be scoped underneath a fixed path. Introduces a new `CourseDirectory.find_assignments` method to help locate assignment directories across steps and students. Fixes #1965
1 parent 99e145b commit 571888e

File tree

12 files changed

+358
-308
lines changed

12 files changed

+358
-308
lines changed

nbgrader/apps/api.py

Lines changed: 22 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
import os
55
import logging
66
import warnings
7+
import typing
8+
from pathlib import Path
79

810
from traitlets.config import LoggingConfigurable, Config, get_config
911
from traitlets import Instance, Enum, Unicode, observe
@@ -123,7 +125,7 @@ def gradebook(self):
123125
"""
124126
return Gradebook(self.coursedir.db_url, self.course_id)
125127

126-
def get_source_assignments(self):
128+
def get_source_assignments(self) -> typing.Set[str]:
127129
"""Get the names of all assignments in the `source` directory.
128130
129131
Returns
@@ -132,29 +134,14 @@ def get_source_assignments(self):
132134
A set of assignment names
133135
134136
"""
135-
filenames = glob.glob(self.coursedir.format_path(
136-
self.coursedir.source_directory,
137-
student_id='.',
138-
assignment_id='*'))
139-
140-
assignments = set([])
141-
for filename in filenames:
142-
# skip files that aren't directories
143-
if not os.path.isdir(filename):
144-
continue
145137

146-
# parse out the assignment name
147-
regex = self.coursedir.format_path(
148-
self.coursedir.source_directory,
149-
student_id='.',
150-
assignment_id='(?P<assignment_id>.*)',
151-
escape=True)
152-
153-
matches = re.match(regex, filename)
154-
if matches:
155-
assignments.add(matches.groupdict()['assignment_id'])
156-
157-
return assignments
138+
return {
139+
entry['assignment_id'] for entry in
140+
self.coursedir.find_assignments(
141+
nbgrader_step=self.coursedir.source_directory,
142+
student_id=".",
143+
)
144+
}
158145

159146
def get_released_assignments(self):
160147
"""Get the names of all assignments that have been released to the
@@ -194,32 +181,14 @@ def get_submitted_students(self, assignment_id):
194181
A set of student ids
195182
196183
"""
197-
# get the names of all student submissions in the `submitted` directory
198-
filenames = glob.glob(self.coursedir.format_path(
199-
self.coursedir.submitted_directory,
200-
student_id='*',
201-
assignment_id=assignment_id))
202184

203-
students = set([])
204-
for filename in filenames:
205-
# skip files that aren't directories
206-
if not os.path.isdir(filename):
207-
continue
208-
209-
# parse out the student id
210-
if assignment_id == "*":
211-
assignment_id = ".*"
212-
regex = self.coursedir.format_path(
213-
self.coursedir.submitted_directory,
214-
student_id='(?P<student_id>.*)',
215-
assignment_id=assignment_id,
216-
escape=True)
217-
218-
matches = re.match(regex, filename)
219-
if matches:
220-
students.add(matches.groupdict()['student_id'])
221-
222-
return students
185+
return {
186+
entry['student_id'] for entry in
187+
self.coursedir.find_assignments(
188+
nbgrader_step=self.coursedir.submitted_directory,
189+
assignment_id=assignment_id
190+
)
191+
}
223192

224193
def get_submitted_timestamp(self, assignment_id, student_id):
225194
"""Gets the timestamp of a submitted assignment.
@@ -243,10 +212,7 @@ def get_submitted_timestamp(self, assignment_id, student_id):
243212
student_id,
244213
assignment_id))
245214

246-
timestamp_pth = os.path.join(assignment_dir, 'timestamp.txt')
247-
if os.path.exists(timestamp_pth):
248-
with open(timestamp_pth, 'r') as fh:
249-
return parse_utc(fh.read().strip())
215+
return self.coursedir.get_existing_timestamp(assignment_dir)
250216

251217
def get_autograded_students(self, assignment_id):
252218
"""Get the ids of students whose submission for a given assignment
@@ -439,21 +405,14 @@ def get_notebooks(self, assignment_id):
439405

440406
# if it doesn't exist in the database
441407
else:
442-
sourcedir = self.coursedir.format_path(
443-
self.coursedir.source_directory,
444-
student_id='.',
445-
assignment_id=assignment_id)
446-
escaped_sourcedir = self.coursedir.format_path(
408+
sourcedir = Path(self.coursedir.format_path(
447409
self.coursedir.source_directory,
448410
student_id='.',
449-
assignment_id=assignment_id,
450-
escape=True)
411+
assignment_id=assignment_id))
451412

452413
notebooks = []
453-
for filename in glob.glob(os.path.join(sourcedir, "*.ipynb")):
454-
regex = re.escape(os.path.sep).join([escaped_sourcedir, "(?P<notebook_id>.*).ipynb"])
455-
matches = re.match(regex, filename)
456-
notebook_id = matches.groupdict()['notebook_id']
414+
for filename in sourcedir.glob("*.ipynb"):
415+
notebook_id = filename.stem
457416
notebooks.append({
458417
"name": notebook_id,
459418
"id": None,

nbgrader/converters/autograde.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -181,18 +181,18 @@ def _init_preprocessors(self) -> None:
181181
for pp in preprocessors:
182182
self.exporter.register_preprocessor(pp)
183183

184-
def convert_single_notebook(self, notebook_filename: str) -> None:
184+
def convert_single_notebook(self, notebook_filename: str, assignment_id: str, student_id: str) -> None:
185185
self.log.info("Sanitizing %s", notebook_filename)
186186
self._sanitizing = True
187187
self._init_preprocessors()
188-
super(Autograde, self).convert_single_notebook(notebook_filename)
188+
super(Autograde, self).convert_single_notebook(notebook_filename, assignment_id, student_id)
189189

190190
notebook_filename = os.path.join(self.writer.build_directory, os.path.basename(notebook_filename))
191191
self.log.info("Autograding %s", notebook_filename)
192192
self._sanitizing = False
193193
self._init_preprocessors()
194194
try:
195195
with utils.setenv(NBGRADER_EXECUTION='autograde'):
196-
super(Autograde, self).convert_single_notebook(notebook_filename)
196+
super(Autograde, self).convert_single_notebook(notebook_filename, assignment_id, student_id)
197197
finally:
198198
self._sanitizing = True

0 commit comments

Comments
 (0)