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

changes to core/power #5062

Closed
smichr opened this issue Jun 18, 2010 · 7 comments
Closed

changes to core/power #5062

smichr opened this issue Jun 18, 2010 · 7 comments

Comments

@smichr
Copy link
Member

smichr commented Jun 18, 2010

This patch tries to get power's eval_power and as_numer_denom() to do more with separation of fractional bases.

Original issue for #5062: http://code.google.com/p/sympy/issues/detail?id=1963
Original author: https://code.google.com/u/117933771799683895267/
Original owner: https://code.google.com/u/117933771799683895267/

@smichr
Copy link
Member Author

smichr commented Jun 18, 2010

It's branch 1963 (single commit over master) at smichr's acct at github.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1963#c1
Original author: https://code.google.com/u/117933771799683895267/

@asmeurer
Copy link
Member

I'm not particularly fond of 

eq=eqn(npos, dpos, pow);assert eq.is_Pow and eq.as_numer_denom() == (npos**pow, dpos**pow)
eq=eqn(npos, dneg, pow);assert eq.is_Pow and eq.as_numer_denom() == (eq, 1)
eq=eqn(nneg, dpos, pow);assert eq.is_Pow and eq.as_numer_denom() == (nneg**pow, dpos**pow)
eq=eqn(nneg, dneg, pow);assert eq.is_Pow and eq.as_numer_denom() == (eq, 1)

I prefer 

eq = eqn(npos, dpos, pow)
assert eq.is_Pow and eq.as_numer_denom() == (npos**pow, dpos**pow)
eq = eqn(npos, dneg, pow)
assert eq.is_Pow and eq.as_numer_denom() == (eq, 1)
eq = eqn(nneg, dpos, pow)
assert eq.is_Pow and eq.as_numer_denom() == (nneg**pow, dpos**pow)
eq = eqn(nneg, dneg, pow)
assert eq.is_Pow and eq.as_numer_denom() == (eq, 1)

or 
eq1 = eqn(npos, dpos, pow)
eq2 = eqn(npos, dneg, pow)
eq3 = eqn(nneg, dpos, pow)
eq4 = eqn(nneg, dneg, pow)

assert eq1.is_Pow and eq1.as_numer_denom() == (npos**pow, dpos**pow)
assert eq2.is_Pow and eq2.as_numer_denom() == (eq2, 1)
assert eq3.is_Pow and eq3.as_numer_denom() == (nneg**pow, dpos**pow)
assert eq4.is_Pow and eq4.as_numer_denom() == (eq4, 1)

Either way, there should be a space on either side of =.  

As far as the changes go, can you explain why (-1/(2 - 3**(1/2)))**(-2*any) behaves different than (-1/(1 - 3**(1/2)))**(-2*any)?

**Labels:** -NeedsReview NeedsBetterPatch  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1963#c2
Original author: https://code.google.com/u/asmeurer@gmail.com/

@smichr
Copy link
Member Author

smichr commented Jun 19, 2010

I will make those changes.

Regarding the behavior, it's (neg/neg)**x vs (neg/pos)**x. Unless sign simplification is done to change the (neg/neg)**x to (pos/pos)**x one can't break up the numerator and denominator...and as_numer_denom isn't attempting simplification. Perhaps it should at least attempt sign simplification? What do you think?

Original comment: http://code.google.com/p/sympy/issues/detail?id=1963#c3
Original author: https://code.google.com/u/117933771799683895267/

@smichr
Copy link
Member Author

smichr commented Jun 19, 2010

Both changes have been made and repushed. (A little clean-up was also done in test_expr.py.) If both num and den are negative they are negated. That seems like the right thing to do since a request is being made for the numrator and denominator...the fraction should be returned separated if possible.

**Labels:** -NeedsBetterPatch NeedsReview  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1963#c4
Original author: https://code.google.com/u/117933771799683895267/

@asmeurer
Copy link
Member

asmeurer commented Sep 6, 2010

**Labels:** smichr  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1963#c5
Original author: https://code.google.com/u/asmeurer@gmail.com/

@smichr
Copy link
Member Author

smichr commented Dec 23, 2010

https://github.com/sympy/sympy/pull/55

Original comment: http://code.google.com/p/sympy/issues/detail?id=1963#c6
Original author: https://code.google.com/u/117933771799683895267/

@smichr
Copy link
Member Author

smichr commented Mar 9, 2011

**Status:** Fixed  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1963#c7
Original author: https://code.google.com/u/117933771799683895267/

@smichr smichr self-assigned this Mar 7, 2014
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants