-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Fix pow and unary operators order #1723
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
base: stable
Are you sure you want to change the base?
Conversation
e22a0d0 to
9ab3dc6
Compare
|
The changelog entry should be much clearer on what is changing since this affects how these expressions are evaluated. |
|
There is still one bug pointed out in #1720 that would not be fixed by this pull request: expression: minimal reproducible examplefrom jinja2 import Environment
env = Environment()
expression = '(-1) ** x'
env.globals = {"x": 2}
jinja_output = env.from_string(f"{{{{ {expression} }}}}").render()
python_output = str(eval(f"{expression}",env.globals))
assert python_output == jinja_outputsuggested fixOne possible fix could be to modify the self.write("(")
self.visit(node.left, frame)
self.write(f" {op} ")
self.visit(node.right, frame)changed into: self.write("((")
self.visit(node.left, frame)
self.write(")")
self.write(f" {op} ")
self.visit(node.right, frame) |
| def test_pow_associative_from_left_to_right(self, env): | ||
| tmpl = env.from_string("{{ 2 ** 3 ** 2 }}") | ||
| assert tmpl.render() == "512" | ||
|
|
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.
In order to verify if issue #1720 is fixed, I would add the following tests:
def test_pow_negative_base(self, env):
tmpl = env.from_string("{{ (- 1) ** 2 }}")
assert tmpl.render() == "1"
def test_pow_negative_base_variable_exponent(self, env):
tmpl = env.from_string("{{ (- 1) ** x }}", {"x": 2})
assert tmpl.render() == "1"
def test_negative_pow_variable_exponent(self, env):
tmpl = env.from_string("{{ - 1 ** x }}", {"x": 2})
assert tmpl.render() == "-1"All the tests seems to pass adding the change suggested here
| .. versionchanged:: 3.2 | ||
| From 3.2, this operator has the same evaluation order as in Python. ``{{ 2 ** 3 ** 2 }}`` will be evaluated as ``2 ** (3 ** 2)``. | ||
|
|
||
| Unlike Python, chained pow is evaluated left to right. |
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.
@davidism, should I remove this outdated explanation? Or it will be better to leave it here in order to keep history in docs?
bart-nouveau
left a comment
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.
It's ok for me, it fixes issues :issue:1720 :issue:`1722
Change order of evaluating expressions with pow by Parser.
Checklist:
CHANGES.rstsummarizing the change and linking to the issue... versionchanged::entries in any relevant code docs.pre-commithooks and fix any issues.pytestandtox, no tests failed.