Skip to content
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

(sin(x)/cos(x)).rewrite(tan(x)) should give tan(x) #16499

Open
oscarbenjamin opened this issue Mar 30, 2019 · 3 comments
Open

(sin(x)/cos(x)).rewrite(tan(x)) should give tan(x) #16499

oscarbenjamin opened this issue Mar 30, 2019 · 3 comments
Labels

Comments

@oscarbenjamin
Copy link
Contributor

Rewrite tan could be improved here:

In [39]: expr = sin(x)/cos(x)                                                                                                                  

In [40]: expr                                                                                                                                  
Out[40]: 
sin(x)
──────
cos(x)

In [41]: expr.rewrite(tan)                                                                                                                     
Out[41]: 
       ⎛x⎞ 
  2tan⎜─⎟ 
       ⎝2⎠ 
───────────
       2⎛x⎞
1 - tan ⎜─⎟
        ⎝2

I would like that to be just tan(x).

@sylee957 sylee957 added the core label Mar 30, 2019
@sylee957
Copy link
Member

sylee957 commented Mar 30, 2019

I see the rewriting works first by rewriting its elements

sympy/sympy/core/basic.py

Lines 1641 to 1644 in a7e6f09

if hints.get('deep', True):
args = [a._eval_rewrite(pattern, rule, **hints)
if isinstance(a, Basic) else a
for a in self.args]

because deep=True by default.
And besides the fact that there is no individual rewriting method implemented in Mul, this may prevent any strategies to be implemented in top level.

I have one trivial example using pattern matching though,

>>> from sympy import *

>>> a = Symbol('a')
>>> expr = cos(a)/sin(a)

>>> x = Wild('x')
>>> expr = expr.replace(sin(x)/cos(x), tan(x))
>>> expr = expr.replace(cos(x)/sin(x), 1/tan(x))
>>> pprint(expr, use_unicode=False)
  1   
------
tan(a)

But I suppose they are already handled better in simplify, if whatever we can introduce for rewriting, can have possibility to be computationally expensive.

@oscarbenjamin
Copy link
Contributor Author

Maybe a high-level Expr._eval_rewrite(tan) can call replace like that before recursing down.

@oscarbenjamin
Copy link
Contributor Author

Also:

In [44]: e                                                                                                                                     
Out[44]: 
cos(x)
──────
sin(x)

In [45]: e.rewrite(cot)                                                                                                                        
Out[45]: 
   2⎛x⎞    
cot ⎜─⎟ - 12⎠    
───────────
       ⎛x⎞ 
  2cot⎜─⎟ 
       ⎝2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants