Repository: sympy/sympy
Pull request: Improve Custom Function Precedence Printing
<!-- Your title above should be a short description of what
was changed. Do not include the issue number in the title. -->

#### References to other Issues or PRs
<!-- If this pull request fixes an issue, write "Fixes #NNNN" in that exact
format, e.g. "Fixes #1234" (see
https://tinyurl.com/auto-closing for more information). Also, please
write a comment on that issue linking back to this pull request once it is
open. -->


#### Brief description of what is fixed or changed
Improves the precedence handling of SymPy with custom functions in multiplication to ensure correct parenthesization and preservation of precedence.

#### Other comments
Problem -
The current implementation fails to correctly parenthesize expressions involving negative numbers during multiplication.

Solution - Introduces a custom printer that intelligently handles parenthesization for custom function expressions.

Test Cases - 
Added comprehensive test for custom function precedence

Fixes #25026

#### Release Notes

<!-- Write the release notes for this release below between the BEGIN and END
statements. The basic format is a bulleted list with the name of the subpackage
and the release note for this PR. For example:

* solvers
  * Added a new solver for logarithmic equations.

* functions
  * Fixed a bug with log of integers. Formerly, `log(-x)` incorrectly gave `-log(x)`.

* physics.units
  * Corrected a semantical error in the conversion between volt and statvolt which
    reported the volt as being larger than the statvolt.

or if no release note(s) should be included use:

NO ENTRY

See https://github.com/sympy/sympy/wiki/Writing-Release-Notes for more
information on how to write release notes. The bot will check your release
notes automatically to see if they are formatted correctly. -->

<!-- BEGIN RELEASE NOTES -->
NO ENTRY
<!-- END RELEASE NOTES -->


Problem statement:
Printing multiplication by negative number with custom (infix) function not correctly parenthesized.
I am trying to define a custom function with lower precedence than multiplication. However, whenever there is a negative number involved, the parentheses are not there anymore.

-----

**Custom function:**
```python
class F(sympy.Function):
    precedence = 45

    def _sympystr(self, printer):
        return f"{printer._print(self.args[0])} F {printer._print(self.args[1])}"
```

**Printing:**
```python
>>> a, b = sympy.symbols("a b")

>>> 2 * F(a, b)
2*(a F b)

>>> -2 * F(a, b)
-2*a F b  # <-------------- No parentheses!
          # Expected: -2*(a F b)
```

-----

Although I am able to get around that, this is a bug, right?

Hints from issue discussion:
Maybe a call to `printer.parenthesize(arg, level)` is missing, but I can't quite get it to work.
Ok, so I debugged this to:
```pycon
>>> precedence(Mul(Integer(-1), Integer(2), F(Symbol('a'), Symbol('b'))))
40
>>> precedence(Mul(Integer(2), F(Symbol('a'), Symbol('b'))))
50
```
so if you set your precedence lower than Add, e.g. 35, it does work:
```pycon
>>> str(-2*F(a,b))
-2*(a F b)
```
not sure if this is to be considered a workaround or "expected" behavior. (I do find the precedence machinery somewhat convoluted...)
Hmm. I guess that could work.
The problem I see, however, is the way `_print_Mul` gets its precedence.

https://github.com/sympy/sympy/blob/9817721589a769319c22437c116d2deffce26aef/sympy/printing/str.py#L263

This function returns the `Add` precedence due to the leading negative number.
Thank you, anyway.
Yup, and here's where that's happening:
https://github.com/sympy/sympy/blob/9817721589a769319c22437c116d2deffce26aef/sympy/printing/precedence.py#L61-L64

It's been like that [forever](https://github.com/sympy/sympy/blame/598726524de3271c8718e5848a64e1e56dd8aa4a/sympy/core/mul.py#L303), it's not immediately apparent to me why but probably unary minus operator interacting with binary operators or something along those lines.
@sympy-bot Assigned me this issue
@bjodah @oscarbenjamin  I have resolved this issue please look into this #27510 
This issue has been resolved by me initially please review it. It has been fixed @bjodah @oscarbenjamin

Commit message: Merge branch 'sympy:master' into preced

Patch to review:
diff --git a/sympy/printing/precedence.py b/sympy/printing/precedence.py
index 563a04b3494a..6c45215ae72b 100644
--- a/sympy/printing/precedence.py
+++ b/sympy/printing/precedence.py
@@ -59,6 +59,16 @@
 
 
 def precedence_Mul(item):
+    from sympy.core.function import Function
+    custom_func_args = [
+        arg for arg in item.args
+        if hasattr(arg, 'precedence') and isinstance(arg, Function)
+    ]
+    if custom_func_args:
+        min_custom_precedence = min(arg.precedence for arg in custom_func_args)
+        if min_custom_precedence < PRECEDENCE["Mul"]:
+            return PRECEDENCE["Mul"]
+
     if item.could_extract_minus_sign():
         return PRECEDENCE["Add"]
     return PRECEDENCE["Mul"]
diff --git a/sympy/printing/tests/test_precedence.py b/sympy/printing/tests/test_precedence.py
index 372a5b0356b7..31d8832acd59 100644
--- a/sympy/printing/tests/test_precedence.py
+++ b/sympy/printing/tests/test_precedence.py
@@ -1,3 +1,4 @@
+import sympy
 from sympy.concrete.products import Product
 from sympy.concrete.summations import Sum
 from sympy.core.function import Derivative
@@ -87,3 +88,45 @@ def test_And_Or():
     assert precedence(x & y) == PRECEDENCE["And"]
     assert precedence(x | y) == PRECEDENCE["Or"]
     assert precedence(~y) == PRECEDENCE["Not"]
+
+
+def test_custom_function_precedence_comparison():
+    """
+    Test cases for custom functions with different precedence values,
+    specifically handling:
+    1. Functions with precedence < PRECEDENCE["Mul"] (50)
+    2. Functions with precedence = Func (70)
+
+    Key distinction:
+    - Lower precedence functions (45) need parentheses: -2*(x F y)
+    - Higher precedence functions (70) don't: -2*x F y
+    """
+    class LowPrecedenceF(sympy.Function):
+        precedence = PRECEDENCE["Mul"] - 5
+        def _sympystr(self, printer):
+            return f"{printer._print(self.args[0])} F {printer._print(self.args[1])}"
+
+    class HighPrecedenceF(sympy.Function):
+        precedence = PRECEDENCE["Func"]
+        def _sympystr(self, printer):
+            return f"{printer._print(self.args[0])} F {printer._print(self.args[1])}"
+
+    def test_low_precedence():
+        expr1 = 2 * LowPrecedenceF(x, y)
+        assert str(expr1) == "2*(x F y)"
+
+        expr2 = -2 * LowPrecedenceF(x, y)
+        assert str(expr2) == "-2*(x F y)"
+
+    def test_high_precedence():
+        expr1 = 2 * HighPrecedenceF(x, y)
+        assert str(expr1) == "2*x F y"
+
+        expr2 = -2 * HighPrecedenceF(x, y)
+        assert str(expr2) == "-2*x F y"
+
+    test_low_precedence()
+    test_high_precedence()
+
+if __name__ == "__main__":
+    test_custom_function_precedence_comparison()


Code review comments:
diff_hunk: @@ -87,3 +88,45 @@ def test_And_Or():
     assert precedence(x & y) == PRECEDENCE["And"]
     assert precedence(x | y) == PRECEDENCE["Or"]
     assert precedence(~y) == PRECEDENCE["Not"]
+
+
+def test_custom_function_precedence_comparison():
+    """
+    Test cases for custom functions with different precedence values,
+    specifically handling:
+    1. Functions with precedence < PRECEDENCE["Mul"] (50)
+    2. Functions with precedence = Func (70)
+
+    Key distinction:
+    - Lower precedence functions (45) need parentheses: -2*(x F y)
+    - Higher precedence functions (70) don't: -2*x F y
+    """
+    class LowPrecedenceF(sympy.Function):
+        precedence = PRECEDENCE["Mul"] - 5
+        def _sympystr(self, printer):
+            return f"{printer._print(self.args[0])} F {printer._print(self.args[1])}"
+
+    class HighPrecedenceF(sympy.Function):
+        precedence = PRECEDENCE["Func"]
+        def _sympystr(self, printer):
+            return f"{printer._print(self.args[0])} F {printer._print(self.args[1])}"
+
+    def test_low_precedence():
+        expr1 = 2 * LowPrecedenceF(x, y)
+        assert str(expr1) == "2*(x F y)"
+
+        expr2 = -2 * LowPrecedenceF(x, y)
+        assert str(expr2) == "-2*(x F y)"
+
+    def test_high_precedence():
+        expr1 = 2 * HighPrecedenceF(x, y)
+        assert str(expr1) == "2*x F y"
+
+        expr2 = -2 * HighPrecedenceF(x, y)
+        assert str(expr2) == "-2*x F y"
+
+    test_low_precedence()
+    test_high_precedence()
+
+if __name__ == "__main__":
path: sympy/printing/tests/test_precedence.py
text: @user1:
should remove the main

@author:
Removed the main

diff_hunk: @@ -59,6 +59,16 @@
 
 
 def precedence_Mul(item):
+    from sympy.core.function import Function
+    custom_func_args = [
path: sympy/printing/precedence.py
text: @user1:
perhaps the new logic could short circuit like

```python
if any(arg.precedence < PRECEDENCE["MUL"] for ... if hasattr ...
```

@author:
Updated the condition of custom precedence

diff_hunk: @@ -1,3 +1,4 @@
+import sympy
path: sympy/printing/tests/test_precedence.py
text: @user1:
should be able to remove

@author:
Removed the import