Opened 4 years ago

Closed 4 years ago

#14694 closed enhancement (fixed)

Update SymPy to 0.7.3

Reported by: eviatarbach Owned by: jdemeyer
Priority: major Milestone: sage-5.12
Component: packages: standard Keywords:
Cc: fbissey Merged in: sage-5.12.beta5
Authors: Eviatar Bach Reviewers: François Bissey, Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Attachments (5)

trac14694_2.patch (1.5 KB) - added by eviatarbach 4 years ago.
trac14694_3.patch (3.1 KB) - added by eviatarbach 4 years ago.
trac14694_3.2.patch (3.1 KB) - added by eviatarbach 4 years ago.
trac14694_4.patch (3.5 KB) - added by eviatarbach 4 years ago.
trac14694_5.patch (3.5 KB) - added by eviatarbach 4 years ago.

Download all attachments as: .zip

Change History (38)

comment:1 Changed 4 years ago by fbissey

  • Cc fbissey added

comment:2 Changed 4 years ago by eviatarbach

  • Description modified (diff)
  • Summary changed from Update SymPy to 0.7.2 to Update SymPy to 0.7.3

SymPy? is now on version 0.7.3, so that's what the Sage version should be updated to.

comment:3 Changed 4 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:4 Changed 4 years ago by eviatarbach

  • Description modified (diff)

comment:5 Changed 4 years ago by eviatarbach

I created the new SPKG, it's up at http://www.phas.ubc.ca/~eviatarb/sympy-0.7.3.spkg.

The patch fixes the only doctest failure I could find.

comment:6 Changed 4 years ago by eviatarbach

  • Status changed from new to needs_review

comment:7 Changed 4 years ago by fbissey

Yes the pretty printing test. There was another one that was broken with 0.7.2 in sage-on-gentoo I will have to chase it to check it is gone away.

Last edited 4 years ago by fbissey (previous) (diff)

comment:8 Changed 4 years ago by fbissey

OK, I check on a vanilla 5.12.beta1 and there is one more doctest failure as with 0.7.2 on sage-on-gentoo:

sage -t --long devel/sage/sage/symbolic/constants.py
**********************************************************************
File "devel/sage/sage/symbolic/constants.py", line 715, in sage.symbolic.constants.NotANumber._sympy_
Failed example:
    sympy.nan == NaN # indirect doctest
Expected:
    True
Got:
    False
**********************************************************************
1 item had failures:
   1 of   3 in sage.symbolic.constants.NotANumber._sympy_

The other patch works fine. We may want to get upstream involved with that one.

comment:9 Changed 4 years ago by eviatarbach

Okay. Do you think we could still merge it though? I don't think users should be comparing SymPy? and Sage objects too often anyway.

comment:10 Changed 4 years ago by fbissey

Sure but the doctest has to be fixed one way or another before we merge. I am not giving a positive review while there is a known doctest breakage.

comment:11 Changed 4 years ago by eviatarbach

Yeah that's what I meant. Patch coming up.

Changed 4 years ago by eviatarbach

comment:12 Changed 4 years ago by eviatarbach

  • Authors set to Eviatar Bach
  • Reviewers set to François Bissey

New patch.

comment:13 Changed 4 years ago by fbissey

  • Status changed from needs_review to positive_review

Not sure what to do with the old doctest but it all passes now. LEt's try to make it for beta4.

comment:14 Changed 4 years ago by jdemeyer

  • Description modified (diff)

comment:15 Changed 4 years ago by jdemeyer

  • Description modified (diff)

comment:16 Changed 4 years ago by fbissey

Sorry forgot to also include the patch in the description. Only the last patch should be applied.

comment:17 Changed 4 years ago by jdemeyer

  • Description modified (diff)

comment:18 Changed 4 years ago by jdemeyer

  • Status changed from positive_review to needs_work
sage -t devel/sage/sage/misc/displayhook.py
**********************************************************************
File "devel/sage/sage/misc/displayhook.py", line 25, in sage.misc.displayhook
Failed example:
    shell.run_cell('integral(x^2/pi^x, x)')
Expected:
     / 2    2                      \  -x*log(pi)
    -\x *log (pi) + 2*x*log(pi) + 2/*e
    --------------------------------------------
                         3
                      log (pi)
Got:
     / 2    2                      \  -x*log(pi) 
    -\x *log (pi) + 2*x*log(pi) + 2/*e           
    ---------------------------------------------
                          3                      
                       log (pi)                  
**********************************************************************
sage -t devel/sage/sage/misc/ascii_art.py
**********************************************************************
File "devel/sage/sage/misc/ascii_art.py", line 807, in sage.misc.ascii_art.ascii_art
Failed example:
    ascii_art(sum(binomial(2*n,n+1)*x^n, n, 0, oo))
Expected:
     /        __________    \
    -\2*x + \/ -4*x + 1  - 1/
    -------------------------
               __________
         2*x*\/ -4*x + 1
Got:
     /        __________    \ 
    -\2*x + \/ -4*x + 1  - 1/ 
    --------------------------
               __________     
         2*x*\/ -4*x + 1      
**********************************************************************

(note the number of hypens)

sage -t devel/sage/sage/symbolic/integration/integral.py
**********************************************************************
File "devel/sage/sage/symbolic/integration/integral.py", line 474, in sage.symbolic.integration.integral.integrate
Failed example:
    (x^y-z).integrate(y,algorithm="sympy")
Exception raised:
    Traceback (most recent call last):
      File "/mazur/release/merger/sage-5.12.beta4/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 479, in _run
        self.execute(example, compiled, test.globs)
      File "/mazur/release/merger/sage-5.12.beta4/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 838, in execute
        exec compiled in globs
      File "<doctest sage.symbolic.integration.integral.integrate[41]>", line 1, in <module>
        (x**y-z).integrate(y,algorithm="sympy")
      File "expression.pyx", line 9759, in sage.symbolic.expression.Expression.integral (sage/symbolic/expression.cpp:40828)
      File "/mazur/release/merger/sage-5.12.beta4/local/lib/python2.7/site-packages/sage/symbolic/integration/integral.py", line 683, in integrate
        return integrator(expression, v, a, b)
      File "/mazur/release/merger/sage-5.12.beta4/local/lib/python2.7/site-packages/sage/symbolic/integration/external.py", line 39, in sympy_integrator
        return result._sage_()
      File "/mazur/release/merger/sage-5.12.beta4/local/lib/python2.7/site-packages/sympy/core/add.py", line 721, in _sage_
        s += x._sage_()
    AttributeError: 'Piecewise' object has no attribute '_sage_'
**********************************************************************
Last edited 4 years ago by jdemeyer (previous) (diff)

comment:19 Changed 4 years ago by fbissey

Hum. I assumed the new patch was correct for the first one on the basis that it was correct in the first version. Not sure how the second one escaped unless eviatarbach used 5.10, it is also specifc to 0.7.3, 0.7.2 just works. The last one is also showing a difference between 0.7.2 and 0.7.3.

comment:20 Changed 4 years ago by eviatarbach

  • Status changed from needs_work to needs_review

Sorry, I should have run testall. New patch fixes all the tests that were failing.

Changed 4 years ago by eviatarbach

Changed 4 years ago by eviatarbach

comment:21 Changed 4 years ago by eviatarbach

  • Description modified (diff)

comment:22 Changed 4 years ago by jdemeyer

  • Status changed from needs_review to needs_work

Why did you change

(x^y-z).integrate(y,algorithm="sympy")

to

(x*y-z).integrate(y,algorithm="sympy")

One needs a good reason to remove doctests.

comment:23 follow-up: Changed 4 years ago by eviatarbach

Because now SymPy? returns a Piecewise object for that integral, which can't be converted back to Sage. Making the conversion work would actually require a change in SymPy?, since currently we look for the _sage_ method in SymPy? objects, which isn't defined for Piecewise.

I couldn't think of anything else to do except modifying the doctest, at least until the conversions are fixed upstream. What do you suggest?

comment:24 in reply to: ↑ 23 Changed 4 years ago by jdemeyer

Replying to eviatarbach:

I couldn't think of anything else to do except modifying the doctest, at least until the conversions are fixed upstream. What do you suggest?

At least, mark the existing doctest # known bug.

comment:25 Changed 4 years ago by fbissey

I would catch instead of changing it so we can know when it is fixed. In any case I will contact sympy upstream shortly about it.

comment:26 Changed 4 years ago by fbissey

comment:27 Changed 4 years ago by asmeurer

I might repeat my request here for people to help out adding _sage_ methods to unsupported objects in SymPy?. The number of SymPy? users who know the corresponding Sage methods is rather low, unfortunately. But SymPy? currently has a lot of special functions and special objects like Piecewise which won't convert directly (even for the special functions, a direct name conversion might not work because of convention differences).

comment:28 Changed 4 years ago by asmeurer

Also, feel free to CC me on any SymPy? related issue in this issue tracker.

Changed 4 years ago by eviatarbach

comment:29 Changed 4 years ago by eviatarbach

  • Status changed from needs_work to needs_review

Thank you Aaron, I will see if I can submit a pull request adding _sage_ methods. It would be good for SymPy? and Sage to interface better to each other.

New patch added. How does this one look?

comment:30 Changed 4 years ago by jdemeyer

When marking a test # known bug, you should add the doctest result what you want to get, not what you actually get. In other words, just add # known bug without changing the result of the test.

comment:31 Changed 4 years ago by eviatarbach

Okay. I removed the # known bug then, since now SymPy? returns a piecewise function and -y*z + x^y/log(x) is no longer the "expected" output. How SymPy?'s Piecewise is converted to Sage's piecewise is left to be implemented, so it's not clear exactly what the expected output will be.

Changed 4 years ago by eviatarbach

comment:32 Changed 4 years ago by eviatarbach

  • Description modified (diff)

comment:33 Changed 4 years ago by jdemeyer

  • Merged in set to sage-5.12.beta5
  • Resolution set to fixed
  • Reviewers changed from François Bissey to François Bissey, Jeroen Demeyer
  • Status changed from needs_review to closed
Note: See TracTickets for help on using tickets.