search

sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.39k stars 473 forks source link

Update doctests for compatibility with ipython 8.x #33170

Closed antonio-rojas closed 2 years ago

antonio-rojas commented 2 years ago

Besides trivial output format changes, the only breakage seems to be tracebacks no longer showing cython source code

**********************************************************************
File "src/sage/repl/interpreter.py", line 77, in sage.repl.interpreter
Failed example:
    print("dummy line"); shell.run_cell('1/0') # see #25320 for the reason of the `...` and the dummy line in this test
Expected:
    dummy line
    ...
    ZeroDivisionError...Traceback (most recent call last)
    <ipython-input-...> in <module>...
    ----> 1 Integer(1)/Integer(0)
    .../sage/rings/integer.pyx in sage.rings.integer.Integer...div...
    ...
    -> ...                  raise ZeroDivisionError("rational division by zero")
       ...            x = <Rational> Rational.__new__(Rational)
       ...            mpq_div_zz(x.value, ....value, (<Integer>right).value)
    <BLANKLINE>
    ZeroDivisionError: rational division by zero
Got:
    dummy line
    ---------------------------------------------------------------------------
    ZeroDivisionError                         Traceback (most recent call last)
    Input In [1], in <module>
    ----> 1 Integer(1)/Integer(0)
    <BLANKLINE>
    File /usr/lib/python3.10/site-packages/sage/rings/integer.pyx:1987, in sage.rings.integer.Integer.__truediv__ (build/cythonized/sage/rings/integer.c:13679)()
    <BLANKLINE>
    ZeroDivisionError: rational division by zero
**********************************************************************

The update itself requires a bunch of new dependencies

CC: @mkoeppe @kiwifb @collares

Component: packages: standard

Author: Antonio Rojas

Branch/Commit: 4d2b53f

Reviewer: Gonzalo Tornaría

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

antonio-rojas commented 2 years ago

Branch: u/arojas/update_to_ipython_8

antonio-rojas commented 2 years ago

Commit: c373551

antonio-rojas commented 2 years ago

Description changed:

--- 
+++ 
@@ -1 +1,33 @@
+Besides trivial output format changes, the only breakage seems to be tracebacks no longer showing cython source code

+```
+**********************************************************************
+File "src/sage/repl/interpreter.py", line 77, in sage.repl.interpreter
+Failed example:
+    print("dummy line"); shell.run_cell('1/0') # see #25320 for the reason of the `...` and the dummy line in this test
+Expected:
+    dummy line
+    ...
+    ZeroDivisionError...Traceback (most recent call last)
+    <ipython-input-...> in <module>...
+    ----> 1 Integer(1)/Integer(0)
+    .../sage/rings/integer.pyx in sage.rings.integer.Integer...div...
+    ...
+    -> ...                  raise ZeroDivisionError("rational division by zero")
+       ...            x = <Rational> Rational.__new__(Rational)
+       ...            mpq_div_zz(x.value, ....value, (<Integer>right).value)
+    <BLANKLINE>
+    ZeroDivisionError: rational division by zero
+Got:
+    dummy line
+    ---------------------------------------------------------------------------
+    ZeroDivisionError                         Traceback (most recent call last)
+    Input In [1], in <module>
+    ----> 1 Integer(1)/Integer(0)
+    <BLANKLINE>
+    File /usr/lib/python3.10/site-packages/sage/rings/integer.pyx:1987, in sage.rings.integer.Integer.__truediv__ (build/cythonized/sage/rings/integer.c:13679)()
+    <BLANKLINE>
+    ZeroDivisionError: rational division by zero
+**********************************************************************
+```
+The update itself requires a bunch of new dependencies
antonio-rojas commented 2 years ago

New commits:

c373551Fix tests expected output to match ipython 8
017dfe3e-cef2-4b91-ad1f-232b5e7b9d6f commented 2 years ago
comment:3

(Thank you for maintaining Sage on archlinux! Just updated to 9.5)

I think it might be a good idea to document somewhere how to disable the autoformatter and suggestion in IPython 8, which are enabled by default (also useful for people who haven't updated to IPython 8 and want to enable these features)

shell = get_ipython()
shell.autoformatter = None
shell.pt_app.auto_suggest = None
# from prompt_toolkit.auto_suggest import AutoSuggestFromHistory
# shell.autoformatter = "black"
# shell.pt_app.auto_suggest = AutoSuggestFromHistory()
mkoeppe commented 2 years ago
comment:4

According to release notes, IPython 8 is still in alpha/beta stage, https://ipython.readthedocs.io/en/stable/whatsnew/version8.html#ipython-8-0

So I don't think we should do the actual update soon. Perhaps open a separate ticket for the update and only put doctest adjustments on the present ticket?

yyyyx4 commented 2 years ago
comment:5

Replying to @8d1h:

I think it might be a good idea to document somewhere how to disable the autoformatter and suggestion in IPython 8, which are enabled by default

In my opinion, we should actually disable these features by default. They change Sage's behavior in a potentially very irritating way and many users won't even know what is causing it and which keywords to search for to change it back.

collares commented 2 years ago
comment:6

I filed https://github.com/alexmojaki/stack_data/issues/21 to address the lack of Cython source code in tracebacks. I also asked for a clarification with regards to IPython 8's stability and, according to the developer, IPython 8 is a stable release; the notice in the release notes about it being alpha/beta quality just wasn't removed soon enough.

collares commented 2 years ago
comment:7

Although I won't have time to test it until next week, upstream just reported that Cython source code should now appear on tracebacks if Python package stack_data is updated to 0.2.0.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

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

a90a314Fix cython source test output for ipython 8 syntax
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Changed commit from c373551 to a90a314

antonio-rojas commented 2 years ago
comment:9

Replying to @collares:

Although I won't have time to test it until next week, upstream just reported that Cython source code should now appear on tracebacks if Python package stack_data is updated to 0.2.0.

Confirmed, thanks. Test still failed because of slightly different syntax, should be fixed now.

tornaria commented 2 years ago
comment:10

In addition to the failure reported in the description (which a90a314 fixes) I'm getting two more doctest failures when upgrading to ipython 8.0.1:

$ sage -t src/sage/repl/interpreter.py src/sage/repl/interface_magic.py 
Running doctests with ID 2022-02-28-15-14-42-bbd14288.
Using --optional=build,pip,sage,sage_spkg,void
Features to be detected: 4ti2,benzene,bliss,buckygen,conway_polynomials,csdp,database_cremona_ellcurve,database_cremona_mini_ellcurve,database_jones_numfield,database_knotinfo,dvipng,graphviz,imagemagick,jupymake,kenzo,latte_int,lrslib,mcqd,meataxe,pandoc,pdf2svg,plantri,pynormaliz,rubiks,sage.combinat,sage.geometry.polyhedron,sage.graphs,sage.plot,sage.rings.number_field,sage.rings.real_double,sage.symbolic,sage_numerical_backends_coin,sagemath_doc_html,sphinx,tdlib
Doctesting 2 files.
sage -t --warn-long 32.0 --random-seed=265810042777151130630499733658806769461 src/sage/repl/interpreter.py
**********************************************************************
File "src/sage/repl/interpreter.py", line 425, in sage.repl.interpreter.SagePreparseTransformer
Failed example:
    shell.run_cell(bad_syntax)
Expected:
      File "<string>", line unknown
    SyntaxError: Mismatched ']'
    <BLANKLINE>
Got:
      File <string>
    SyntaxError: Mismatched ']'
    <BLANKLINE>
**********************************************************************
1 item had failures:
   1 of  14 in sage.repl.interpreter.SagePreparseTransformer
    [137 tests, 1 failure, 3.36 s]
sage -t --warn-long 32.0 --random-seed=265810042777151130630499733658806769461 src/sage/repl/interface_magic.py
**********************************************************************
File "src/sage/repl/interface_magic.py", line 262, in sage.repl.interface_magic.InterfaceMagic.cell_magic_factory
Failed example:
    shell.run_cell('%%gap foo\n1+1;\n')
Expected:
    ...File "<string>", line unknown
    SyntaxError: Interface magics have no options, got "foo"
    <BLANKLINE>
Got:
    Traceback (most recent call last):
    <BLANKLINE>
      File /usr/lib/python3.10/site-packages/IPython/core/interactiveshell.py:3251 in run_code
        exec(code_obj, self.user_global_ns, self.user_ns)
    <BLANKLINE>
      Input In [1] in <module>
        get_ipython().run_cell_magic('gap', 'foo', '1+1;\n')
    <BLANKLINE>
      File /usr/lib/python3.10/site-packages/IPython/core/interactiveshell.py:2257 in run_cell_magic
        result = fn(*args, **kwargs)
    <BLANKLINE>
      File /usr/lib/python3.10/site-packages/sage/repl/interface_magic.py:295 in cell_magic
        raise SyntaxError('Interface magics have no options, got "{0}"'.format(line))
    <BLANKLINE>
      File <string>
    SyntaxError: Interface magics have no options, got "foo"
    <BLANKLINE>
**********************************************************************
1 item had failures:
   1 of  11 in sage.repl.interface_magic.InterfaceMagic.cell_magic_factory
    [30 tests, 1 failure, 1.56 s]
----------------------------------------------------------------------
sage -t --warn-long 32.0 --random-seed=265810042777151130630499733658806769461 src/sage/repl/interpreter.py  # 1 doctest failed
sage -t --warn-long 32.0 --random-seed=265810042777151130630499733658806769461 src/sage/repl/interface_magic.py  # 1 doctest failed
----------------------------------------------------------------------
Total time for all tests: 15.6 seconds
    cpu time: 3.0 seconds
    cumulative wall time: 4.9 seconds
Features detected for doctesting:

The following patch fixes these:

diff --git a/src/sage/repl/interface_magic.py b/src/sage/repl/interface_magic.py
index 8a455b69b0e..95d89628992 100644
--- a/src/sage/repl/interface_magic.py
+++ b/src/sage/repl/interface_magic.py
@@ -260,7 +260,7 @@ class InterfaceMagic(object):
             2
             120
             sage: shell.run_cell('%%gap foo\n1+1;\n')
-            ...File "<string>", line unknown
+            ...File ...<string>...
             SyntaxError: Interface magics have no options, got "foo"
             <BLANKLINE>
             sage: shell.run_cell('%%gap?')
diff --git a/src/sage/repl/interpreter.py b/src/sage/repl/interpreter.py
index cabb71097bc..47adea3979e 100644
--- a/src/sage/repl/interpreter.py
+++ b/src/sage/repl/interpreter.py
@@ -423,7 +423,7 @@ def SagePreparseTransformer(lines):
         sage: from sage.repl.interpreter import get_test_shell
         sage: shell = get_test_shell()
         sage: shell.run_cell(bad_syntax)
-          File "<string>", line unknown
+          File ...<string>...
         SyntaxError: Mismatched ']'
         <BLANKLINE>
         sage: shell.quit()
antonio-rojas commented 2 years ago
comment:11

Replying to @tornaria:

In addition to the failure reported in the description (which a90a314 fixes) I'm getting two more doctest failures when upgrading to ipython 8.0.1:

This is fixed in the branch already

tornaria commented 2 years ago
comment:12

Replying to @antonio-rojas:

Replying to @tornaria:

In addition to the failure reported in the description (which a90a314 fixes) I'm getting two more doctest failures when upgrading to ipython 8.0.1:

This is fixed in the branch already

My bad, it's there indeed as c373551, thanks.

Sorry for the noise.

tornaria commented 2 years ago
comment:14

This works ok with ipython 8.0.1, but it's already broken on ipython 8.1.0 and 8.1.1:

Failed example:
    print("dummy line"); shell.run_cell('1/0') # see #25320 for the reason of the `...` and the dummy line in this test
Expected:
    dummy line
    ...
    ZeroDivisionError...Traceback (most recent call last)
    ...in <module>...
    ----> 1 Integer(1)/Integer(0)
    .../sage/rings/integer.pyx... in sage.rings.integer.Integer...div...
    ...
    -> ...                  raise ZeroDivisionError("rational division by zero")
       ...            x = <Rational> Rational.__new__(Rational)
       ...            mpq_div_zz(x.value, ....value, (<Integer>right).value)
    <BLANKLINE>
    ZeroDivisionError: rational division by zero
Got:
    dummy line
    ---------------------------------------------------------------------------
    ZeroDivisionError                         Traceback (most recent call last)
    Input In [1], in <cell line: 1>()
    ----> 1 Integer(1)/Integer(0)
    <BLANKLINE>
    File /usr/lib/python3.10/site-packages/sage/rings/integer.pyx:1987, in sage.rings.integer.Integer.__truediv__ (build/cythonized/sage/rings/integer.c:13693)()
       1985 if type(left) is type(right):
       1986     if mpz_sgn((<Integer>right).value) == 0:
    -> 1987         raise ZeroDivisionError("rational division by zero")
       1988     x = <Rational> Rational.__new__(Rational)
       1989     mpq_div_zz(x.value, (<Integer>left).value, (<Integer>right).value)
    <BLANKLINE>
    ZeroDivisionError: rational division by zero
**********************************************************************

Can you add the following patch on top of your previous commits?

--- a/src/sage/repl/interpreter.py
+++ b/src/sage/repl/interpreter.py
@@ -78,7 +78,7 @@ Check that Cython source code appears in tracebacks::
     dummy line
     ...
     ZeroDivisionError...Traceback (most recent call last)
-    ...in <module>...
+    ...
     ----> 1 Integer(1)/Integer(0)
     .../sage/rings/integer.pyx... in sage.rings.integer.Integer...div...
     ...

The other file (interface_magic.py) is still ok with ipython 8.1.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Changed commit from a90a314 to 4d2b53f

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

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

4d2b53fUpdate test for ipython 8.1
antonio-rojas commented 2 years ago
comment:17

Done, thanks. Needs review since this is now only about fixing tests.

mkoeppe commented 2 years ago

Author: Antonio Rojas

tornaria commented 2 years ago
comment:19

Thanks, I'll give positive review once CI on https://github.com/void-linux/void-packages/pull/36209 is done.

tornaria commented 2 years ago
comment:20

All ok on void linux using system ipython 8.1 (glibc 64 and 32 bit, musl 64 bit tested at https://github.com/void-linux/void-packages/pull/36209)

tornaria commented 2 years ago

Reviewer: Gonzalo Tornaría

mkoeppe commented 2 years ago
comment:21

Replying to @collares:

I also asked for a clarification with regards to IPython 8's stability and, according to the developer, IPython 8 is a stable release; the notice in the release notes about it being alpha/beta quality just wasn't removed soon enough.

I've opened #33530 for the actual update. This is for Sage 9.7 because the update will require us to drop support for Python 3.7.

vbraun commented 2 years ago

Changed branch from u/arojas/update_to_ipython_8 to 4d2b53f