Counterintuitive (wrong?) parenthesis when combining exp() and powers

[jonbarron]

Description

When you latexify something like log((x/c)^2), the rendering you get looks (to me at least) like it's log(x/c)^2:

In contrast, Wolfram Alpha for example will insert extra parens to disambiguate this case.

Reproduction

Describe how to reproduce the issue by other people.

@latexify.function()
def f(x, c):
  return np.log((x/c)**2)
f

[odashi]

Yeah I know this kind of ambiguity still remains around unary operators, including bracket-less functions as you reported above. We haven't figure out the correct rules around this yet.
Could you provide more examples that we should treat?

[xiziqiao]

Hi, there. I am really interested in this project. Could I participate in solving this minor issue?

[odashi]

@xiziqiao Yeah please describe the ideas if you have it already.

I want to avoid simply adding brackets to all functions because of its redundancy, but currently I have no clear policy to appropriately choose whether add a bracket or not.

[xiziqiao]

@odashi Hi Mr. Odashi, thanks for responding. Our thoughts now is pretty straightforward. If we have a input like log x^2, then the output latex should be (log x)^2. If we want the latex output to be log(x^2), then the python input should be log（（x^2））.
Did I illustrate my idea clearly? We can communicate by email if you have any questions. Additional thoughts and advice are much appreciated. My email would be [email protected].

[odashi]

@xiziqiao Please keep discussion on this thread to avoid hidden decisions.

The library has several rules to avoid parentheses around the argument of several functions to follow the normal math notation. That means the abbreviation is the special case (=most functions are exactly followed by the parentheses), and this issue is eventually related to whether re-considering this rule or not.

[odashi]

I didn't remember soon about the context around the pow operator, but it looks it should be treated similarly to that of other binary operators?

[tesknight]

how about just add a parentheses when it is found a pow expression is taken as arg in a function call? I think only the pow operator need a special treatment as its precedence is higher than unary function call

[xiziqiao]

@odashi I am thinking the same thing as @tesknight. We can simply add a parenthese when pow function is put into the function call.

[odashi]

It looks the cause comes from the custom precedence of function calls. It is set as a smaller value than pow, resulting in removal of brackets between a unary function and pow.

latexify_py/src/latexify/codegen/expression_rules.py

Line 56 in 1cfb8e7

_CALL_PRECEDENCE = _PRECEDENCES[ast.UAdd] + 1

I think this rule can be removed so that unary functions of a non-constant always require brackets.

[odashi]

Yeah removing the rule works:

[odashi]

Unfortunately it brings another bug:

[xiziqiao]

Okay. This seems neat. Are you going to keep changing this rule? If so I will take a look of this new bug

[odashi]

I think the above change shouldn't be applied, and we should change the behavior around the generation rules of Call instead:

latexify_py/src/latexify/codegen/expression_codegen.py

Lines 409 to 421 in 1cfb8e7

	if rule.is_unary and len(node.args) == 1:
	# Unary function. Applies the same wrapping policy with the unary operators.
	# NOTE(odashi):
	# Factorial "x!" is treated as a special case: it requires both inner/outer
	# parentheses for correct interpretation.
	precedence = expression_rules.get_precedence(node)
	arg = node.args[0]
	force_wrap = isinstance(arg, ast.Call) and (
	func_name == "factorial"
	or ast_utils.extract_function_name_or_none(arg) == "factorial"
	)
	arg_latex = self._wrap_operand(arg, precedence, force_wrap)
	elements = [rule.left, arg_latex, rule.right]

I'm working on this, will propose some appropriate changes.

[tesknight]

I agree. Simply make pow a higher precedence or unary function call a higher precedence both bring some bugs. We may need to force wrap its arg when a Pow expression be its arg

[tesknight]

I think we can make pow and call have the same precedence and let force_wrap work when they were taken as arg of each other.
I make such changes and it pass the examples you provide above:

[odashi]

I made a similar change: #195
The difference is that I just added the pow flag in visit_Call, and removed the same-precedence rule in force_wrap of _wrap_operand since I found that this is not actually effective in the current implementation.