Update to pynac-0.6.9

[rwst]

Changelog:

• fix bug affecting Cygwin compile (pynac does not build on Cygwin #21265)
• combine some numerics in nested powers; (Doctest: Numerics in nested symbolic powers not combined #21360)
• fix and clean up ex::coefficients() (rewrite buggy Expression.coefficients() without Maxima #20455)
• expand() denominators too (Symbolic fractions not expanded #21302)
• make ex::combine_fractions() recursive (Option to combine symbolic fractions recursively #20858)
• more normal() options (Extend normalize() and use it instead of Maxima in simplify_rational() #21335)
• Appell F1 function skeleton
• updates from GiNaC

https://github.com/pynac/pynac/releases/download/pynac-0.6.9/pynac-0.6.9.tar.bz2

CC: @embray @tscrim @kiwifb

Component: packages: standard

Author: Ralf Stephan, Erik Bray, Jeroen Demeyer

Branch: b06cfa0

Reviewer: Jeroen Demeyer, Ralf Stephan

Issue created by migration from https://trac.sagemath.org/ticket/21369

[rwst]

Branch: u/rws/update_to_pynac_0_6_9

[rwst]

Commit: a4f58d7

[rwst]

New commits:

b996f4f	version/chksum
91a08d2	changes affecting Sage behaviour or interface
a4f58d7	doctest fixes

[jdemeyer]

comment:3

This is a regression (based on a doctest you removed):

Before:

sage: k = GF(7); expand((k(1)*x^2 + k(1)*x)^7)
x^14 + x^7

After:

sage: k = GF(7); expand((k(1)*x^2 + k(1)*x)^7)
x^14 + 0*x^13 + 0*x^12 + 0*x^11 + 0*x^10 + 0*x^9 + 0*x^8 + x^7

[jdemeyer]

comment:4

In general, for any doctest that you remove, it should be clearly justified why that doctest can be removed.

[rwst]

comment:5

Oh well, an impasse. My justification to me personally is that I do not want to fix a feature of symbolics that is better served by algebraic structures in Sage. As it is buggy already now, and we see no complaints, I also suspect that symbolic Mod is used by a negligible number of people.

I offer to start a sage-devel thread, and to work on a deprecation ticket in case others agree.

[sagetrac-git]

Changed commit from a4f58d7 to ac7d971

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

ac7d971

revert removal of pos.char. doctests; add "known bug"

[tscrim]

comment:7

For # known bug, you should put in the correct answer.

[jdemeyer]

comment:8

Replying to @tscrim:

For # known bug, you should put in the correct answer.

That is the case here if I'm not mistaken.

[jdemeyer]

comment:9

Replying to @rwst:

I offer to start a sage-devel thread

Please do!

[tscrim]

comment:10

Replying to @jdemeyer:

Replying to @tscrim:

For # known bug, you should put in the correct answer.

That is the case here if I'm not mistaken.

Ah, yes; sorry, I flopped comment:3.

[rwst]

comment:11

https://groups.google.com/forum/?hl=en#!topic/sage-devel/auLwQWtpnz0

[jdemeyer]

comment:13

Thanks!

[jdemeyer]

comment:14

1. The changes to simplify_rational() seem more than just a Pynac update. For easier review, I suggest to move those changes to Extend normalize() and use it instead of Maxima in simplify_rational() #21335 and focus on the upgrade.

2. Why this?

@@ -1387,10 +1387,19 @@ cdef class Expression(CommutativeRingElement):
             ...
             TypeError: unable to simplify to float approximation
         """
+        from sage.functions.other import real, imag
         try:
-            return float(self._eval_self(float))
+            ret = float(self._eval_self(float))
         except TypeError:
-            raise TypeError("unable to simplify to float approximation")
+            try:
+                c = (self._eval_self(complex))
+                if imag(c) == 0:
+                    ret = real(c)
+                else:
+                    raise
+            except TypeError:
+                raise TypeError("unable to simplify to float approximation")
+        return ret
 
     def __complex__(self):

I understand and see that it makes sense, I am just wondering which bug it fixes.

3. The code mentioned in 2. could be improved a bit. Instead of using the ret variable, I suggest just returning the value directly. In the complex case, I would use c = complex(self._eval_self(complex)) and then use the real and imag attributes (not functions).

4. Can you justify

diff --git a/src/sage/symbolic/expression.pyx b/src/sage/symbolic/expression.pyx
index 6c4171f..d780bbc 100644
--- a/src/sage/symbolic/expression.pyx
+++ b/src/sage/symbolic/expression.pyx
@@ -1191,7 +1191,7 @@ cdef class Expression(CommutativeRingElement):
         except TypeError as err:
             # try the evaluation again with the complex field
             # corresponding to the parent R
-            if R is float:
+            if R in (float, complex):
                 R_complex = complex
             else:
                 try:

It seems pointless to have R_complex == R.

[sagetrac-git]

Changed commit from ac7d971 to 3dd8058

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

3dd8058

address reviewer's comments

[rwst]

comment:16

Replying to @jdemeyer:

1. The changes to simplify_rational() seem more than just a Pynac update. For easier review, I suggest to move those changes to Extend normalize() and use it instead of Maxima in simplify_rational() #21335 and focus on the upgrade.

OK.

2. Why this?

...
I understand and see that it makes sense, I am just wondering which bug it fixes.

sage: float(sqrt(2)/sqrt(abs(-(I - 1)*sqrt(2) - I - 1)))
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-9-5bba6e8d1a16> in <module>()
----> 1 float(sqrt(Integer(2))/sqrt(abs(-(I - Integer(1))*sqrt(Integer(2)) - I - Integer(1))))

/home/ralf/sage/src/sage/symbolic/expression.pyx in sage.symbolic.expression.Expression.__float__ (build/cythonized/sage/symbolic/expression.cpp:10473)()
   1391             return float(self._eval_self(float))
   1392         except TypeError:
-> 1393             raise TypeError("unable to simplify to float approximation")
   1394
   1395     def __complex__(self):

TypeError: unable to simplify to float approximation

I was unable to find out why this bombed but it does so at least since Sage-7.1.

4. Can you justify

+            if R in (float, complex):

It seems pointless to have R_complex == R.

Yes but this also prevents the else branch done (R.complex_field() with R being complex).

---

New commits:

3dd8058

address reviewer's comments

---

New commits:

3dd8058

address reviewer's comments

[sagetrac-git]

Changed commit from 3dd8058 to 1f29305

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

1f29305

add doctest

[jdemeyer]

comment:18

Replying to @rwst:

sage: float(sqrt(2)/sqrt(abs(-(I - 1)*sqrt(2) - I - 1)))
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-9-5bba6e8d1a16> in <module>()
----> 1 float(sqrt(Integer(2))/sqrt(abs(-(I - Integer(1))*sqrt(Integer(2)) - I - Integer(1))))

/home/ralf/sage/src/sage/symbolic/expression.pyx in sage.symbolic.expression.Expression.__float__ (build/cythonized/sage/symbolic/expression.cpp:10473)()
   1391             return float(self._eval_self(float))
   1392         except TypeError:
-> 1393             raise TypeError("unable to simplify to float approximation")
   1394
   1395     def __complex__(self):

TypeError: unable to simplify to float approximation

I was unable to find out why this bombed but it does so at least since Sage-7.1.

Can you add this as doctest then?

And delete the line from sage.functions.other import real, imag

[jdemeyer]

comment:19

Replying to @rwst:

Yes but this also prevents the else branch done (R.complex_field() with R being complex).

And why is that a problem? Sure, it will raise an AttributeError, but that's caught. So I still don't see the point.

[jdemeyer]

comment:20

I am happy with the branch on this ticket except for the float/complex stuff in expression.pyx that I need to look at more.

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

0065f62

address reviewer's comments

[sagetrac-git]

Changed commit from 1f29305 to 0065f62

[jdemeyer]

Changed branch from u/rws/update_to_pynac_0_6_9 to u/jdemeyer/update_to_pynac_0_6_9

[jdemeyer]

Changed commit from 0065f62 to b06cfa0

[jdemeyer]

comment:23

I fixed the underlying cause of the float/complex problem which is in _eval_self. This way, no changes to __float__ are needed. If you agree with this, you can set the ticket to positive_review.

---

New commits:

b06cfa0

Fix _eval_self(float) for "real" complex expressions

[jdemeyer]

Reviewer: Jeroen Demeyer

[rwst]

Changed author from Ralf Stephan, Erik M. Bray to Ralf Stephan, Erik M. Bray, Jeroen Demeyer

[rwst]

Changed reviewer from Jeroen Demeyer to Jeroen Demeyer, Ralf Stephan

[rwst]

comment:24

Is fine and passes tests. Thanks.

[tscrim]

comment:25

This also built without problems on Cygwin32/64 for me.

[vbraun]

Changed branch from u/jdemeyer/update_to_pynac_0_6_9 to b06cfa0

[fchapoton]

Changed author from Ralf Stephan, Erik M. Bray, Jeroen Demeyer to Ralf Stephan, Erik Bray, Jeroen Demeyer

[fchapoton]

Changed commit from b06cfa0 to none