Commit 9ff401330259b2f642614cbd94296f4e4aa3f32c
1 parent
b7b63323
Exists in
master
and in
1 other branch
add checks to:
- duplicate ref in the same file - options file in radio and checkbox questions load_yaml can raise more useful exceptions
Showing
4 changed files
with
46 additions
and
26 deletions
Show diff stats
BUGS.md
aprendizations/learnapp.py
... | ... | @@ -90,7 +90,6 @@ class LearnApp(object): |
90 | 90 | for c, d in self.courses.items(): |
91 | 91 | for goal in d['goals']: |
92 | 92 | if goal not in self.deps.nodes(): |
93 | - # logger.error(f'Goal "{goal}" of "{c}"" not in the graph') | |
94 | 93 | raise LearnException(f'Goal "{goal}" from course "{c}" ' |
95 | 94 | ' does not exist') |
96 | 95 | |
... | ... | @@ -409,14 +408,24 @@ class LearnApp(object): |
409 | 408 | fullpath: str = path.join(topicpath, t['file']) |
410 | 409 | |
411 | 410 | logger.debug(f' Loading {fullpath}') |
412 | - questions: List[QDict] = load_yaml(fullpath, default=[]) | |
411 | + # questions: List[QDict] = load_yaml(fullpath, default=[]) | |
412 | + try: | |
413 | + questions: List[QDict] = load_yaml(fullpath) | |
414 | + except Exception: | |
415 | + raise LearnException(f'Failed to load "{fullpath}"') | |
413 | 416 | |
414 | 417 | # update refs to include topic as prefix. |
415 | 418 | # refs are required to be unique only within the file. |
416 | 419 | # undefined are set to topic:n, where n is the question number |
417 | 420 | # within the file |
421 | + localrefs = set() # refs in current file | |
418 | 422 | for i, q in enumerate(questions): |
419 | 423 | qref = q.get('ref', str(i)) # ref or number |
424 | + if qref in localrefs: | |
425 | + msg = f'Duplicate ref "{qref}" in "{topicpath}"' | |
426 | + raise LearnException(msg) | |
427 | + localrefs.add(qref) | |
428 | + | |
420 | 429 | q['ref'] = f'{tref}:{qref}' |
421 | 430 | q['path'] = topicpath |
422 | 431 | q.setdefault('append_wrong', t['append_wrong']) | ... | ... |
aprendizations/questions.py
... | ... | @@ -74,7 +74,14 @@ class QuestionRadio(Question): |
74 | 74 | def __init__(self, q: QDict) -> None: |
75 | 75 | super().__init__(q) |
76 | 76 | |
77 | - n = len(self['options']) | |
77 | + try: | |
78 | + n = len(self['options']) | |
79 | + except KeyError: | |
80 | + msg = f'Missing `options` in radio question. See {self["path"]}' | |
81 | + raise QuestionException(msg) | |
82 | + except TypeError: | |
83 | + msg = f'`options` must be a list. See {self["path"]}' | |
84 | + raise QuestionException(msg) | |
78 | 85 | |
79 | 86 | self.set_defaults(QDict({ |
80 | 87 | 'text': '', |
... | ... | @@ -185,7 +192,14 @@ class QuestionCheckbox(Question): |
185 | 192 | def __init__(self, q: QDict) -> None: |
186 | 193 | super().__init__(q) |
187 | 194 | |
188 | - n = len(self['options']) | |
195 | + try: | |
196 | + n = len(self['options']) | |
197 | + except KeyError: | |
198 | + msg = f'Missing `options` in radio question. See {self["path"]}' | |
199 | + raise QuestionException(msg) | |
200 | + except TypeError: | |
201 | + msg = f'`options` must be a list. See {self["path"]}' | |
202 | + raise QuestionException(msg) | |
189 | 203 | |
190 | 204 | # set defaults if missing |
191 | 205 | self.set_defaults(QDict({ | ... | ... |
aprendizations/tools.py
... | ... | @@ -111,14 +111,13 @@ class HighlightRenderer(mistune.Renderer): |
111 | 111 | |
112 | 112 | def table(self, header, body): |
113 | 113 | return ('<table class="table table-sm"><thead class="thead-light">' |
114 | - f'{header}</thead><tbody>{body}</tbody></table>') | |
114 | + f'{header}</thead><tbody>{body}</tbody></table>') | |
115 | 115 | |
116 | 116 | def image(self, src, title, alt): |
117 | 117 | alt = mistune.escape(alt, quote=True) |
118 | 118 | title = mistune.escape(title or '', quote=True) |
119 | 119 | return (f'<img src="/file/{src}" alt="{alt}" title="{title}"' |
120 | - 'class="img-fluid">') | |
121 | - # class="img-fluid mx-auto d-block" | |
120 | + f'class="img-fluid">') # class="img-fluid mx-auto d-block" | |
122 | 121 | |
123 | 122 | # Pass math through unaltered - mathjax does the rendering in the browser |
124 | 123 | def block_math(self, text): |
... | ... | @@ -150,25 +149,22 @@ def load_yaml(filename: str, default: Any = None) -> Any: |
150 | 149 | filename = path.expanduser(filename) |
151 | 150 | try: |
152 | 151 | f = open(filename, 'r', encoding='utf-8') |
153 | - except FileNotFoundError: | |
154 | - logger.error(f'Cannot open "{filename}": not found') | |
155 | - except PermissionError: | |
156 | - logger.error(f'Cannot open "{filename}": no permission') | |
157 | - except OSError: | |
158 | - logger.error(f'Cannot open file "{filename}"') | |
159 | - else: | |
160 | - with f: | |
161 | - try: | |
162 | - default = yaml.safe_load(f) | |
163 | - except yaml.YAMLError as e: | |
164 | - if hasattr(e, 'problem_mark'): | |
165 | - mark = e.problem_mark | |
166 | - logger.error(f'File "{filename}" near line {mark.line+1}, ' | |
167 | - f'column {mark.column+1}') | |
168 | - else: | |
169 | - logger.error(f'File "{filename}"') | |
170 | - finally: | |
171 | - return default | |
152 | + except Exception as e: | |
153 | + logger.error(e) | |
154 | + if default is not None: | |
155 | + return default | |
156 | + else: | |
157 | + raise | |
158 | + | |
159 | + with f: | |
160 | + try: | |
161 | + return yaml.safe_load(f) | |
162 | + except yaml.YAMLError as e: | |
163 | + logger.error(str(e).replace('\n', ' ')) | |
164 | + if default is not None: | |
165 | + return default | |
166 | + else: | |
167 | + raise | |
172 | 168 | |
173 | 169 | |
174 | 170 | # --------------------------------------------------------------------------- | ... | ... |