mitodl / mitx-grading-library

The MITx Grading Library, a python grading library for edX
https://edge.edx.org/courses/course-v1:MITx+grading-library+examples/
BSD 3-Clause "New" or "Revised" License
14 stars 9 forks source link

rewrite expression parser/evaluator #134

Closed ChristopherChudzicki closed 6 years ago

ChristopherChudzicki commented 6 years ago

Rewrote much of calc.py and changed name to expressions.py

Noteable changes:

  1. I tried to separate parsing and evaluation as much as possible. Usage now looks like:

    parser = MathParser()
    expr_1 = parser.parse('x + y')
    expr_2 = parser.parse('x*y')
    
    variables_A = { 'x': 5, 'y': 7 }
    variables_B = {'x': -3, 'y': 4}
    
    expr_1.eval(variables_A, {}, {}, {})
    expr_1.eval(variables_B, {}, {}, {}) # evaluate the same expression with different variables, do not re-parse
    
    expr_2.eval(variables_A, {}, {}, {})
    expr_2.eval(variables_B, {}, {}, {}) # evaluate the same expression with different variables, do not re-parse
  2. I changed the grammar so that the parser accepts all alphabetic suffixes and '%'. Advantages:
    • Allows us to give better error messages for things like 'x + 5y'. We used to just say "Unable to parse". Now the expression DOES parse, but we can say "y not allowed directly after number, did you forget *?"
    • Resolves the weirdness in #132
ChristopherChudzicki commented 6 years ago

@jolyonb This is ready for review.

I had to specify pytest-cov version in requirements.txt. Newest version (2.6) dropped support for Python <3.4. We have the most recent version that supports Python 2.7.

https://pytest-cov.readthedocs.io/en/latest/changelog.html#id1

jolyonb commented 6 years ago

Feel free to mark any unconstructive comments as resolved, btw.

ChristopherChudzicki commented 6 years ago

I believe I've just resolved everything except for the adjacent issue. Just confirm you want that and I can change it, or merge as is.

jolyonb commented 6 years ago

If we don't break any tests by setting adjacent to True, then let's go ahead and do it.

jolyonb commented 6 years ago

Awesome job. This almost feels like it's worthy of a revision number!