-
Notifications
You must be signed in to change notification settings - Fork 104
Capture vars not env #66
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
2bba058
03ff98b
9cac009
74c6dbf
10f79f5
f707d5c
bf628ce
793d7a9
5b39dbf
8ecb89b
5b816a2
55330c2
12c3ce9
874a6ec
48d7e1e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -191,8 +191,8 @@ def test_ModelDesc_from_formula(): | |
| for input in ("y ~ x", parse_formula("y ~ x")): | ||
| eval_env = EvalEnvironment.capture(0) | ||
| md = ModelDesc.from_formula(input, eval_env) | ||
| assert md.lhs_termlist == [Term([EvalFactor("y", eval_env)]),] | ||
| assert md.rhs_termlist == [INTERCEPT, Term([EvalFactor("x", eval_env)])] | ||
| assert md.lhs_termlist == [Term([EvalFactor("y")]),] | ||
| assert md.rhs_termlist == [INTERCEPT, Term([EvalFactor("x")])] | ||
|
|
||
| class IntermediateExpr(object): | ||
| "This class holds an intermediate result while we're evaluating a tree." | ||
|
|
@@ -356,8 +356,7 @@ def _eval_number(evaluator, tree): | |
| "only allowed with **", tree) | ||
|
|
||
| def _eval_python_expr(evaluator, tree): | ||
| factor = EvalFactor(tree.token.extra, evaluator._factor_eval_env, | ||
| origin=tree.origin) | ||
| factor = EvalFactor(tree.token.extra, origin=tree.origin) | ||
| return IntermediateExpr(False, None, False, [Term([factor])]) | ||
|
|
||
| class Evaluator(object): | ||
|
|
@@ -585,16 +584,15 @@ def eval(self, tree, require_evalexpr=True): | |
| "a + <-a**2>", | ||
| ] | ||
|
|
||
| def _assert_terms_match(terms, expected_intercept, expecteds, eval_env): # pragma: no cover | ||
| def _assert_terms_match(terms, expected_intercept, expecteds): # pragma: no cover | ||
| if expected_intercept: | ||
| expecteds = [()] + expecteds | ||
| assert len(terms) == len(expecteds) | ||
| for term, expected in zip(terms, expecteds): | ||
| if isinstance(term, Term): | ||
| if isinstance(expected, str): | ||
| expected = (expected,) | ||
| assert term.factors == tuple([EvalFactor(s, eval_env) | ||
| for s in expected]) | ||
| assert term.factors == tuple([EvalFactor(s) for s in expected]) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It didn't percolate very far off the call stack, but I removed a couple uses. Good catch. |
||
| else: | ||
| assert term == expected | ||
|
|
||
|
|
@@ -609,11 +607,9 @@ def _do_eval_formula_tests(tests): # pragma: no cover | |
| print(model_desc) | ||
| lhs_intercept, lhs_termlist, rhs_intercept, rhs_termlist = result | ||
| _assert_terms_match(model_desc.lhs_termlist, | ||
| lhs_intercept, lhs_termlist, | ||
| eval_env) | ||
| lhs_intercept, lhs_termlist) | ||
| _assert_terms_match(model_desc.rhs_termlist, | ||
| rhs_intercept, rhs_termlist, | ||
| eval_env) | ||
| rhs_intercept, rhs_termlist) | ||
|
|
||
| def test_eval_formula(): | ||
| _do_eval_formula_tests(_eval_tests) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpicks:
"foo" "bar"is the same as"foobar"(and so is"foo" 'bar'for that matter). It's even slightly more efficient, b/c+is evaluated at runtime, while leaving it out gives a single string directly in the .pyc. Not that this efficiency matters at all :-). But as a style/consistency thing I leave the+out.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. I'm aware of this feature, but I vaguely remember being bitten by it (although maybe it was in C instead of Python), so I personally tend to prefer being explicit unless necessary. But given that this is the style of the rest of the code base, I'll go with it. Done.