Skip to content

Commit 5adec72

Browse files
author
Filip Schouwenaars
authored
Merge pull request #156 from datacamp/spec-messages
Spec messages
2 parents 98e3c9f + 19a30d4 commit 5adec72

19 files changed

Lines changed: 208 additions & 148 deletions

pythonwhat/State.py

Lines changed: 58 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import ast
22
import inspect
3+
import string
34
from copy import copy
5+
from functools import partial
46
from pythonwhat.parsing import TargetVars, FunctionParser, ObjectAccessParser, parser_dict
57
from pythonwhat.Reporter import Reporter
68
from pythonwhat.Feedback import Feedback
@@ -9,6 +11,7 @@
911
from pythonwhat.converters import get_manual_converters
1012
from collections.abc import Mapping
1113
from itertools import chain
14+
from jinja2 import Template
1215

1316
class Context(Mapping):
1417
def __init__(self, context=None, prev=None):
@@ -31,6 +34,37 @@ def __len__(self):
3134
return len(self._items)
3235

3336

37+
#class MsgFormatter(string.Formatter):
38+
# def vformat(self, format_string, args, kwargs):
39+
# """Restricted vformat, which does not format entries with converters or format specs"""
40+
# used_args = set()
41+
# result = []
42+
# for chunk in string._string.formatter_parser(format_string):
43+
# orig = self._orig_from_chunk(*chunk)
44+
# # return original string if there are converters or format specs,
45+
# # otherwise, parse as normal
46+
# if chunk[1] and any(chunk[2:]):
47+
# result.append(orig)
48+
# elif chunk[0] and not any(chunk[1:]):
49+
# result.append(chunk[0])
50+
# else:
51+
# res, _ = self._vformat(orig, args, kwargs, used_args, 1)
52+
# result.append(res)
53+
# return "".join(result)
54+
#
55+
# def get_field(self, field_name, args, kwargs):
56+
# try:
57+
# return super().get_field(field_name, args, kwargs)
58+
# except (KeyError, AttributeError):
59+
# return "{"+field_name+"}", "NA"
60+
#
61+
# @staticmethod
62+
# def _orig_from_chunk(literal_text, field_name, format_spec, conversion):
63+
# # of form, literal_str {var_name!conversion:format_spec}
64+
# conversion = '!' + conversion if conversion else ""
65+
# format_spec = ":" + format_spec if format_spec else ""
66+
# return "%s{%s%s%s}"%(literal_text, field_name, conversion, format_spec)
67+
3468
class State(object):
3569
"""State of the SCT environment.
3670
@@ -108,10 +142,17 @@ def build_message(self, tail="", fmt_kwargs=None):
108142
msgs = self.messages[:] + [{'msg': tail or "", 'kwargs':fmt_kwargs}]
109143
# format messages in list, by iterating over previous, current, and next message
110144
for prev_d, d, next_d in zip([{}, *msgs[:-1]], msgs, [*msgs[1:], {}]):
111-
out = d['msg'].format(parent = prev_d.get('kwargs'),
112-
child = next_d.get('kwargs'),
113-
this = d['kwargs'],
114-
**d['kwargs'])
145+
tmp_kwargs = {'parent': prev_d.get('kwargs'),
146+
'child': next_d.get('kwargs'),
147+
'this': d['kwargs'],
148+
**d['kwargs']}
149+
if d['msg'].startswith('FMT:'):
150+
out = d['msg'].replace('FMT:', "").format(**tmp_kwargs)
151+
elif d['msg'].startswith('__JINJA__:'):
152+
out = Template(d['msg'].replace('__JINJA__:', "")).render(**tmp_kwargs)
153+
else:
154+
out = d['msg']
155+
115156
out_list.append(out)
116157

117158
return "".join(out_list)
@@ -120,7 +161,7 @@ def to_child_state(self, student_subtree, solution_subtree,
120161
student_context=None, solution_context=None,
121162
student_parts=None, solution_parts=None,
122163
highlight = None,
123-
append_message=""):
164+
append_message="", node_name=""):
124165
"""Dive into nested tree.
125166
126167
Set the current state as a state with a subtree of this syntax tree as
@@ -154,7 +195,8 @@ def to_child_state(self, student_subtree, solution_subtree,
154195
student_parts = student_parts, solution_parts = solution_parts,
155196
highlight = highlight, messages = messages)
156197

157-
child = State(student_code = utils_ast.extract_text_from_node(self.full_student_code, student_subtree),
198+
klass = State if not node_name else self.SUBCLASSES[node_name]
199+
child = klass(student_code = utils_ast.extract_text_from_node(self.full_student_code, student_subtree),
158200
full_student_code = self.full_student_code,
159201
pre_exercise_code = self.pre_exercise_code,
160202
student_context = student_context,
@@ -238,8 +280,6 @@ def parse_int(x):
238280
# @property
239281
# def student_withs(self): ...
240282
# when defining the State class.
241-
from functools import partial
242-
243283
def getx(tree_name, Parser, ext_attr, self):
244284
"""getter for Parser outputs"""
245285
# return cached output if possible
@@ -249,26 +289,18 @@ def getx(tree_name, Parser, ext_attr, self):
249289
else:
250290
# otherwise, run parser over tree
251291
p = Parser()
292+
# set mappings for parsers that inspect attribute access
293+
if ext_attr != 'mappings' and Parser in [FunctionParser, ObjectAccessParser]:
294+
p.mappings = self.pre_exercise_mappings.copy()
295+
# run parser
252296
p.visit(getattr(self, tree_name))
253297
# cache
254298
self._parser_cache[cache_key] = p
255299
return getattr(p, ext_attr)
256300

257-
def get_func_map(tree_name, ext_attr, self):
258-
"""getter for FunctionParser outputs, uses pre_exercise_mappings"""
259-
cache_key = tree_name + FunctionParser.__name__
260-
if self._parser_cache.get(cache_key):
261-
p = self._parser_cache[cache_key]
262-
else:
263-
p = FunctionParser()
264-
p.mappings = self.pre_exercise_mappings.copy()
265-
p.visit(getattr(self, tree_name))
266-
self._parser_cache[cache_key] = p
267-
return getattr(p, ext_attr)
268-
269301
# put a property getter on state for each parsed ast tree output.
270302
# since the getter takes only one argument, self, partial functions
271-
# are used to set all other arguments on getx and get_func_map
303+
# are used to set all other arguments on getx
272304
for s in ['student', 'solution']:
273305
tree_name = s+'_tree'
274306
for k, Parser in parser_dict.items():
@@ -278,17 +310,16 @@ def get_func_map(tree_name, ext_attr, self):
278310
prop_oa_map = property(partial(getx, tree_name, ObjectAccessParser, 'mappings'))
279311
setattr(State, s+'_oa_mappings', prop_oa_map)
280312

281-
# Getters for FunctionParser -----
282-
# calls
283-
prop_calls = property(partial(get_func_map, tree_name, 'calls'))
284-
setattr(State, s+'_function_calls', prop_calls)
285-
# mappings
286-
prop_map = property(partial(get_func_map, tree_name, 'mappings'))
313+
# mappings from FunctionParser
314+
prop_map = property(partial(getx, tree_name, FunctionParser, 'mappings'))
287315
setattr(State, s+'_mappings', prop_map)
288316

317+
# mappings for pre exercise code from FunctionParser
289318
pec_prop_map = property(partial(getx, 'pre_exercise_tree', FunctionParser, 'mappings'))
290319
setattr(State, 'pre_exercise_mappings', pec_prop_map)
291320

321+
# State subclasses based on parsed output -------------------------------------
322+
State.SUBCLASSES = {node_name: type(node_name, (State,), {}) for node_name in parser_dict}
292323

293324
# global setters on State -----------------------------------------------------
294325
def set_converter(key, fundef):

pythonwhat/check_funcs.py

Lines changed: 36 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
from functools import partial
66
import copy
77

8-
def part_to_child(stu_part, sol_part, append_message, state):
8+
def part_to_child(stu_part, sol_part, append_message, state, node_name=None):
99
# stu_part and sol_part will be accessible on all templates
1010
append_message['kwargs'].update({'stu_part': stu_part, 'sol_part': sol_part})
1111

@@ -15,13 +15,13 @@ def part_to_child(stu_part, sol_part, append_message, state):
1515
stu_part.get('target_vars'), sol_part.get('target_vars'),
1616
stu_part, sol_part,
1717
highlight = stu_part.get('highlight'),
18-
append_message = append_message)
18+
append_message = append_message, node_name=node_name)
1919

2020
# otherwise, assume they are just nodes
2121
return state.to_child_state(stu_part, sol_part, append_message = append_message)
2222

2323

24-
def check_part(name, part_msg, state=None, missing_msg="", expand_msg=""):
24+
def check_part(name, part_msg, state=None, missing_msg="Are you sure it's defined?", expand_msg=""):
2525
"""Return child state with name part as its ast tree"""
2626
rep = Reporter.active_reporter
2727

@@ -36,17 +36,19 @@ def check_part(name, part_msg, state=None, missing_msg="", expand_msg=""):
3636
return part_to_child(stu_part, sol_part, append_message, state)
3737

3838
def check_part_index(name, index, part_msg,
39-
missing_msg="Define more {part}.",
39+
missing_msg="FMT:Are you sure it is defined?",
4040
state=None, expand_msg=""):
4141
"""Return child state with indexed name part as its ast tree"""
4242

4343
rep = Reporter.active_reporter
4444

4545
# create message
4646
ordinal = "" if isinstance(index, str) else get_ord(index+1)
47+
fmt_kwargs = {'index': index, 'ordinal': ordinal}
48+
fmt_kwargs['part'] = part_msg.format(**fmt_kwargs)
4749

4850
append_message = {'msg': expand_msg,
49-
'kwargs': {'part': part_msg, 'index': index, 'ordinal': ordinal}}
51+
'kwargs': fmt_kwargs}
5052

5153
# check there are enough parts for index
5254
stu_parts = state.student_parts[name]
@@ -62,16 +64,17 @@ def check_part_index(name, index, part_msg,
6264
# return child state from part
6365
return part_to_child(stu_part, sol_part, append_message, state)
6466

65-
MSG_MISSING = "The system wants to check the {ordinal} {typestr} you defined but hasn't found it."
66-
MSG_PREPEND = "Check your code in the {child[part]} of the {ordinal} {typestr}. "
67+
MSG_MISSING = "FMT:The system wants to check the {typestr} you defined but hasn't found it."
68+
MSG_PREPEND = "__JINJA__:Check your code in the {{child['part']+ ' of the' if child['part']}} {{typestr}}. "
6769
def check_node(name, index, typestr, missing_msg=MSG_MISSING, expand_msg=MSG_PREPEND, state=None):
6870
rep = Reporter.active_reporter
6971
stu_out = getattr(state, 'student_'+name)
7072
sol_out = getattr(state, 'solution_'+name)
7173

7274
# check if there are enough nodes for index
7375
fmt_kwargs = {'ordinal': get_ord(index+1) if isinstance(index, int) else "",
74-
'typestr': typestr}
76+
'index': index}
77+
fmt_kwargs['typestr'] = typestr.format(**fmt_kwargs)
7578

7679
# test if node can be indexed succesfully
7780
try: stu_out[index]
@@ -87,7 +90,10 @@ def check_node(name, index, typestr, missing_msg=MSG_MISSING, expand_msg=MSG_PRE
8790
'kwargs': fmt_kwargs
8891
}
8992

90-
return part_to_child(stu_part, sol_part, append_message, state)
93+
return part_to_child(stu_part, sol_part, append_message, state, node_name=name)
94+
95+
96+
# Part tests ------------------------------------------------------------------
9197

9298
def has_part(name, msg, state=None, fmt_kwargs=None):
9399
rep = Reporter.active_reporter
@@ -112,9 +118,8 @@ def has_equal_part(name, msg, state):
112118
'sol_part': state.solution_parts,
113119
'name': name}
114120

115-
if d['stu_part'][name] != d['sol_part'][name]:
116-
_msg = state.build_message(msg, d)
117-
rep.do_test(Test(Feedback(_msg, state.highlight)))
121+
_msg = state.build_message(msg, d)
122+
rep.do_test(EqualTest(d['stu_part'][name], d['sol_part'][name], Feedback(_msg, state.highlight)))
118123

119124
return state
120125

@@ -221,11 +226,11 @@ def set_context(*args, state=None, **kwargs):
221226
student_context = out_stu, solution_context = out_sol)
222227

223228

224-
def check_arg(name, missing_msg='check the argument `{part}`, ', state=None):
229+
def check_args(name, missing_msg='FMT:Are you sure it is defined?', state=None):
225230
if name in ['*args', '**kwargs']:
226231
return check_part(name, name, state=state, missing_msg = missing_msg)
227232
else:
228-
return check_part_index('args', name, name, state=state, missing_msg = missing_msg)
233+
return check_part_index('args', name, "argument `%s`"%name, state=state, missing_msg = missing_msg)
229234

230235

231236
# CALL CHECK ==================================================================
@@ -281,14 +286,14 @@ def run_call(args, node, process, get_func, **kwargs):
281286
return get_func(process = process, tree=func_expr, call = fmt_args, **kwargs)
282287

283288

284-
MSG_CALL_INCORRECT = "Calling it should result in {str_sol}, instead got {str_sol}"
285-
MSG_CALL_ERROR = "Calling it should result in {str_sol}, instead got an error"
289+
MSG_CALL_INCORRECT = "FMT:Calling it should result in {str_sol}, instead got {str_sol}"
290+
MSG_CALL_ERROR = "FMT:Calling it should result in {str_sol}, instead got an error"
286291
def call(args,
287292
test='value',
288293
incorrect_msg=MSG_CALL_INCORRECT,
289294
error_msg=MSG_CALL_ERROR,
290-
# TODO hardcoded lambda description for now
291-
argstr='the Xth lambda function',
295+
# TODO kept for backwards compatibility in test_function_definition/lambda
296+
argstr='',
292297
state=None, **kwargs):
293298
rep = Reporter.active_reporter
294299
test_type = ('value', 'output', 'error')
@@ -299,11 +304,14 @@ def call(args,
299304
eval_sol, str_sol = run_call(args, state.solution_parts['node'], state.solution_process, get_func, **kwargs)
300305

301306
if (test == 'error') ^ isinstance(str_sol, Exception):
302-
_msg_prefix = "Calling %s for arguments %s " % (argstr, args)
303-
raise ValueError(_msg_prefix + call_warnings[test])
307+
_msg = state.build_message("FMT:Calling for arguments {args} resulted in an error (or not an error if testing for one). Error message: {str_sol}",
308+
dict(args=args, str_sol=str_sol))
309+
raise ValueError(_msg)
304310

305311
if isinstance(eval_sol, ReprFail):
306-
raise ValueError("Can't get the result of calling %s for arguments %s: %s" % (argstr, args, eval_sol.info))
312+
_msg = state.build_message("FMT:Can't get the result of calling it for arguments {args}: {eval_sol.info}",
313+
dict(args = args, eval_sol=eval_sol))
314+
raise ValueError(_msg)
307315

308316
# Run for Submission ------------------------------------------------------
309317
eval_stu, str_stu = run_call(args, state.student_parts['node'], state.student_process, get_func, **kwargs)
@@ -323,11 +331,10 @@ def call(args,
323331

324332
# Expression tests ------------------------------------------------------------
325333
from pythonwhat.tasks import ReprFail, UndefinedValue
326-
from pythonwhat.Test import EqualTest
327334
from pythonwhat import utils
328-
def has_expr(incorrect_msg,
335+
def has_expr(incorrect_msg="FMT:Unexpected expression {test}: expected `{sol_eval}`, got `{stu_eval}` with values{extra_env}.",
329336
error_msg="Running an expression in the student process caused an issue",
330-
undefined_msg="Have you defined `{name}` without errors?",
337+
undefined_msg="FMT:Have you defined `{name}` without errors?",
331338
extra_env=None,
332339
context_vals=None,
333340
expr_code=None,
@@ -359,7 +366,8 @@ def has_expr(incorrect_msg,
359366
context = state.solution_context)
360367

361368
if (test == 'error') ^ isinstance(str_sol, Exception):
362-
raise ValueError("evaluating expression raised error in solution process")
369+
raise ValueError("evaluating expression raised error in solution process (or not an error if testing for one). "
370+
"Error message: %s"%str_sol)
363371
if isinstance(eval_sol, ReprFail):
364372
raise ValueError("Couldn't figure out the value of a default argument: " + eval_sol.info)
365373

@@ -368,7 +376,9 @@ def has_expr(incorrect_msg,
368376
context = state.student_context)
369377

370378
# kwargs ---
371-
fmt_kwargs = {'stu_part': state.student_parts, 'sol_part': state.solution_parts, 'name': name}
379+
fmt_kwargs = {'stu_part': state.student_parts, 'sol_part': state.solution_parts,
380+
'name': name, 'test': test,
381+
'extra_env': " "+str(extra_env or ""), 'context_vals': context_vals}
372382
fmt_kwargs['stu_eval'] = utils.shorten_str(str(eval_stu))
373383
fmt_kwargs['sol_eval'] = utils.shorten_str(str(eval_sol))
374384

0 commit comments

Comments
 (0)