Update to pynac-0.6.9

[rws]

Changelog:

• fix bug affecting Cygwin compile (#21265)
• combine some numerics in nested powers; (#21360)
• fix and clean up ex::coefficients() (#20455)
• expand() denominators too (#21302)
• make ex::combine_fractions() recursive (#20858)
• more normal() options (#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

[rws]

New commits:

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

[jdemeyer]

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]

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

[rws]

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.

[git]

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

​ac7d971

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

[tscrim]

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

[jdemeyer]

Replying to tscrim:

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

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

[jdemeyer]

Replying to rws:

I offer to start a sage-devel thread

Please do!

[tscrim]

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.

[rws]

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

[jdemeyer]

Thanks!

[jdemeyer]

1. The changes to simplify_rational() seem more than just a Pynac update. For easier review, I suggest to move those changes to #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

   src/sage/symbolic/expression.pyx

   @@ -1191 +1191 @@
            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.

[git]

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

​3dd8058

address reviewer's comments

[rws]

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 #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

[git]

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

​1f29305

add doctest

[jdemeyer]

Replying to rws:

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]

Replying to rws:

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]

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.

[git]

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

​0065f62

address reviewer's comments

[jdemeyer]

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

[rws]

Is fine and passes tests. Thanks.

[tscrim]

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