okpy / ok-client

A Python client for the OK autograding system
https://okpy.org/
Apache License 2.0
57 stars 42 forks source link

ok does not pass the nosetests #380

Closed kavigupta closed 5 years ago

kavigupta commented 5 years ago

I think I've tracked the error down to https://github.com/okpy/ok-client/commit/b605ed3de5979a3436c4e81b49c6a1c42248456f#diff-c66760f0a6aa193db7eca384468b2da0 by @jathak

@jathak do you remember what you were doing in this PR? I'm pretty sure that ast.literal_eval can't handle the string "[1,2,3,2+2]" since it can't handle binary operations.

python
Python 3.7.2 (default, Dec 29 2018, 06:19:36) 
[GCC 7.3.0] :: Anaconda, Inc. on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import ast
>>> ast.literal_eval("2+2")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/kavi/anaconda3/envs/py37/lib/python3.7/ast.py", line 91, in literal_eval
    return _convert(node_or_string)
  File "/home/kavi/anaconda3/envs/py37/lib/python3.7/ast.py", line 90, in _convert
    return _convert_signed_num(node)
  File "/home/kavi/anaconda3/envs/py37/lib/python3.7/ast.py", line 63, in _convert_signed_num
    return _convert_num(node)
  File "/home/kavi/anaconda3/envs/py37/lib/python3.7/ast.py", line 55, in _convert_num
    raise ValueError('malformed node or string: ' + repr(node))
ValueError: malformed node or string: <_ast.BinOp object at 0x7f32460ee4e0>
jathak commented 5 years ago

IIRC, Ok originally did exact string matching for unlocking questions, which caused issues when students entered responses that were correct, but not in the canonical form (e.g. [1,2,3] instead of [1, 2, 3]). At first, we tried to fix this by evaling the student response (in #101), but that was too lenient (see #211), so #223 replaced eval there with ast.literal_eval, which was the closest thing to expression canonicalization we could find in the standard library.

jathak commented 5 years ago

Perhaps ast.literal_evals semantics changed between Python versions?

kavigupta commented 5 years ago

Ah that must be it since sumukh said "For future reference: ast.literal_eval supports addition/subtraction of integers (but not other operations/other types" on the PR

kavigupta commented 5 years ago

Yep: https://bugs.python.org/issue31778

Ok I'll change the test