Opened 3 years ago

Closed 18 months ago

Last modified 14 months ago

#28197 closed enhancement (fixed)

upgrade to ipython 7

Reported by: chapoton Owned by: jdemeyer, embray
Priority: critical Milestone: sage-9.2
Component: packages: standard Keywords: upgrade
Cc: embray, jdemeyer, slelievre, fbissey, gh-timokau, arojas Merged in:
Authors: Jonathan Kliem, John Palmieri, Antonio Rojas Reviewers: John Palmieri, Matthias Koeppe
Report Upstream: N/A Work issues:
Branch: e21a7b0 (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by mkoeppe)

Upgrade ipython and related packages:

All of these package versions support python >= 3.6 (note #29033).

Tarballs: see checksums.ini [upstream_url]. (To configure Sage to download from the upstream URLs, use ./configure --enable-download-from-upstream-url)

Change History (81)

comment:1 Changed 3 years ago by chapoton

  • Branch set to public/ticket/28197
  • Cc embray jdemeyer slelievre added
  • Commit set to 0d4f0c76fe01226a70ba77652833e04f6823dd19
  • Description modified (diff)

This is basically working, but maybe without preparsing..


New commits:

0d4f0c7adaptation to ipython 7 : first tentative

comment:2 Changed 3 years ago by arojas

  • Cc arojas added

comment:3 Changed 3 years ago by jhpalmieri

Should this depend on #28190?

comment:4 Changed 3 years ago by chapoton

  • Dependencies set to #28190

comment:5 Changed 3 years ago by jipilab

  • Description modified (diff)

comment:6 Changed 3 years ago by jipilab

  • Description modified (diff)

comment:7 Changed 3 years ago by jipilab

  • Description modified (diff)

Current issues (8.9.beta3):

sage: installed_packages()['ipython']                                                                                                                                                                              
'7.6.1'
sage: 2^3                                                                                                                                                                                                          
1
sage: 2**3                                                                                                                                                                                                         
8
sage: n=4                                                                                                                                                                                                          
sage: type(n)                                                                                                                                                                                                      
<class 'int'>

The preparsing is not working since it has been commented out.

comment:8 Changed 3 years ago by jipilab

Trying to launch ipython one get:

jplabbe@blackbomb:~/sage$ sage -ipython
Traceback (most recent call last):
...
pkg_resources.DistributionNotFound: The 'jedi>=0.10' distribution was not found and is required by ipython

So one has to install the jedi module as well.

comment:9 Changed 2 years ago by chapoton

  • Description modified (diff)

comment:10 Changed 2 years ago by fbissey

  • Cc fbissey added

comment:11 Changed 2 years ago by arojas

A patch that makes sage work with ipython 7, including the preparser, is available at [1]. I haven't bothered to make it backwards compatible with older ipython, and it probably needs some polishing, feel free to take it from there.

[1] https://aur.archlinux.org/cgit/aur.git/tree/sagemath-ipython7.patch?h=sagemath-python3-git

comment:12 Changed 2 years ago by embray

  • Component changed from PLEASE CHANGE to refactoring

(For lack of a better component; perhaps we should add a "REPL" component...?)

comment:13 Changed 2 years ago by gh-timokau

  • Cc gh-timokau added

comment:14 Changed 2 years ago by jhpalmieri

  • Component changed from refactoring to packages: standard
  • Owner changed from (none) to jdemeyer, embray

comment:15 Changed 2 years ago by jhpalmieri

This is Python 3 only. Are there specific plans for when we drop Python 2 support from Sage? Version 9.1?

comment:16 Changed 2 years ago by arojas

ipython 7.10 breaks many more tests due to it no longer sorting dicts. Some tests output is actually random now. Updated patch at https://aur.archlinux.org/cgit/aur.git/tree/sagemath-ipython7.patch?h=sagemath-git

comment:17 Changed 2 years ago by embray

  • Milestone changed from sage-8.9 to sage-9.1

Ticket retargeted after milestone closed

comment:18 Changed 2 years ago by gh-mwageringel

  • Description modified (diff)

The new jedi completion engine seems to have some problems. For example with IPython 7.6.1, invoking tab completion like

sage: 1 + <TAB>

causes the interpreter to freeze for about 1.5 minutes (apparently, a largish cache is created at ~/.cache/jedi/). After that, several deprecated functions are imported into global scope causing deprecation warnings to be printed, such as:

Importing absolute_igusa_invariants_kohel from here is deprecated. If you need to use it, please import it directly from sage.schemes.hyperelliptic_curves.invariants
See https://trac.sagemath.org/28064 for details.
  return getattr(handle.access, attribute)(*args, **kwargs)

This also seems to slow down exiting Sage.


As for the display of dictionaries during doctests, I suggest to open a separate ticket to remove the sorting. This can be done independently from the IPython upgrade, once Python 2 support is dropped by Sage (in 9.1 I assume).

comment:19 Changed 2 years ago by chapoton

a preparation step at #28948

comment:20 Changed 2 years ago by arojas

  • Dependencies changed from #28190 to #28190 #29042

I've opened #29042 for the dict sorting changes

comment:21 follow-up: Changed 22 months ago by fbissey

I have the following that I don't really know what to do with

sage -t --long /usr/lib/python3.7/site-packages/sage/repl/ipython_extension.py
**********************************************************************
File "/usr/lib/python3.7/site-packages/sage/repl/ipython_extension.py", line 352, in sage.repl.ipython_extension.SageMagics.cython
Failed example:
    shell.run_cell('''
    %%cython
    def f():
        print('test')
    ''')
Expected nothing
Got:
    UsageError: Line magic function `%%cython` not found.
**********************************************************************
File "/usr/lib/python3.7/site-packages/sage/repl/ipython_extension.py", line 357, in sage.repl.ipython_extension.SageMagics.cython
Failed example:
    f()
Exception raised:
    Traceback (most recent call last):
      File "/usr/lib/python3.7/site-packages/sage/doctest/forker.py", line 681, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/usr/lib/python3.7/site-packages/sage/doctest/forker.py", line 1123, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.repl.ipython_extension.SageMagics.cython[3]>", line 1, in <module>
        f()
    NameError: name 'f' is not defined
**********************************************************************
File "/usr/lib/python3.7/site-packages/sage/repl/ipython_extension.py", line 385, in sage.repl.ipython_extension.SageMagics.fortran
Failed example:
    shell.run_cell('''
    %%fortran
    C FILE: FIB1.F
          SUBROUTINE FIB(A,N)
    C
    C     CALCULATE FIRST N FIBONACCI NUMBERS
    C
          INTEGER N
          REAL*8 A(N)
          DO I=1,N
             IF (I.EQ.1) THEN
                A(I) = 0.0D0
             ELSEIF (I.EQ.2) THEN
                A(I) = 1.0D0
             ELSE
                A(I) = A(I-1) + A(I-2)
             ENDIF
          ENDDO
          END
    C END FILE FIB1.F
    ''')
Expected nothing
Got:
      File "<ipython-input-1-ab346d8f879f>", line 3
        C FILE: FIB1.F
             ^
    SyntaxError: invalid syntax
    <BLANKLINE>
**********************************************************************
File "/usr/lib/python3.7/site-packages/sage/repl/ipython_extension.py", line 406, in sage.repl.ipython_extension.SageMagics.fortran
Failed example:
    fib
Exception raised:
    Traceback (most recent call last):
      File "/usr/lib/python3.7/site-packages/sage/doctest/forker.py", line 681, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/usr/lib/python3.7/site-packages/sage/doctest/forker.py", line 1123, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.repl.ipython_extension.SageMagics.fortran[3]>", line 1, in <module>
        fib
    NameError: name 'fib' is not defined
**********************************************************************
File "/usr/lib/python3.7/site-packages/sage/repl/ipython_extension.py", line 410, in sage.repl.ipython_extension.SageMagics.fortran
Failed example:
    fib(a, 10)
Exception raised:
    Traceback (most recent call last):
      File "/usr/lib/python3.7/site-packages/sage/doctest/forker.py", line 681, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/usr/lib/python3.7/site-packages/sage/doctest/forker.py", line 1123, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.repl.ipython_extension.SageMagics.fortran[6]>", line 1, in <module>
        fib(a, Integer(10))
    NameError: name 'fib' is not defined
**********************************************************************
File "/usr/lib/python3.7/site-packages/sage/repl/ipython_extension.py", line 411, in sage.repl.ipython_extension.SageMagics.fortran
Failed example:
    a
Expected:
    array([  0.,   1.,   1.,   2.,   3.,   5.,   8.,  13.,  21.,  34.])
Got:
    array([0., 1., 2., 3., 4., 5., 6., 7., 8., 9.])
**********************************************************************
2 items had failures:
   2 of   5 in sage.repl.ipython_extension.SageMagics.cython
   4 of   9 in sage.repl.ipython_extension.SageMagics.fortran
    [80 tests, 6 failures, 2.14 s]
----------------------------------------------------------------------
sage -t --long /usr/lib/python3.7/site-packages/sage/repl/ipython_extension.py  # 6 doctests failed

It all comes down from the first test failing because of the %%cython magic function not being found. Any clue as to what to do to fix that?

comment:22 in reply to: ↑ 21 Changed 22 months ago by arojas

Replying to fbissey:

It all comes down from the first test failing because of the %%cython magic function not being found. Any clue as to what to do to fix that?

Broken preparsing? it works fine here with the patch at comment:16

comment:23 Changed 22 months ago by fbissey

Because I was using ipython-7.5 so far I was using the patch at comment:11 (with small variations between sage-9.0 and 9.1.beta). I'll check the preparsing in comment:16.

comment:24 follow-up: Changed 22 months ago by fbissey

I should have asked. When you said it is fine, is it sage-9.0 or 9.1.betaX? I am working on 9.1.beta9 right now.

comment:25 in reply to: ↑ 24 Changed 22 months ago by arojas

Replying to fbissey:

I should have asked. When you said it is fine, is it sage-9.0 or 9.1.betaX? I am working on 9.1.beta9 right now.

Both. The patch for 9.0 is at https://git.archlinux.org/svntogit/community.git/tree/trunk/sagemath-ipython7.patch?h=packages/sagemath

comment:26 Changed 22 months ago by fbissey

Nope. I must be missing something subtle or something in my stack has a bug.

comment:27 Changed 22 months ago by mkoeppe

  • Milestone changed from sage-9.1 to sage-9.2

comment:28 Changed 22 months ago by arojas

  • Dependencies changed from #28190 #29042 to #28190 #29042 #29428

comment:29 Changed 21 months ago by git

  • Commit changed from 0d4f0c76fe01226a70ba77652833e04f6823dd19 to 289e60359f76a1cf804b71e16eaa89fa29dc8029

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

289e603trac 28197: IPython 7.13.0, with new required packages jedi, backcall

comment:30 Changed 21 months ago by jhpalmieri

Here is a branch for IPython 7.13, which requires new packages jedi and backcall, to give people something new to play with, especially since we're getting close to starting the 9.2 release cycle.

I don't see the problem reported at #29428, or at least the small example there doesn't cause problems for me. The preparser doesn't work, though. I'm about to try the patch in comment:16, after which I'll add it to the branch. (Is there a missing or an extra single quote on line 47 of that patch?)

comment:31 Changed 21 months ago by jhpalmieri

  • Description modified (diff)

comment:32 Changed 21 months ago by git

  • Commit changed from 289e60359f76a1cf804b71e16eaa89fa29dc8029 to 31bb55c0494c0538624c37dfb842f74f079da198

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

31bb55ctrac 28197: fix preparser for new IPython: patch taken from

comment:33 follow-up: Changed 21 months ago by jhpalmieri

Here is the preparser patch from comment:16 (although without the lines changing sys.getrecursionlimit()). When merged with #29042, I see these doctest failures:

sage -t --long src/sage/tests/cmdline.py  # 3 doctests failed
sage -t --long src/sage/crypto/mq/sr.py  # 1 doctest failed
sage -t --long src/sage/rings/polynomial/multi_polynomial_sequence.py  # 1 doctest failed
sage -t --long src/sage/misc/package.py  # 1 doctest failed
sage -t --long src/sage/rings/polynomial/pbori.pyx  # 1 doctest failed
sage -t --long src/sage/sat/solvers/dimacs.py  # 1 doctest failed
sage -t --long src/doc/en/reference/sat/index.rst  # 1 doctest failed
sage -t --long src/sage/sat/converters/polybori.py  # 1 doctest failed
sage -t --long src/sage/sat/boolean_polynomials.py  # 1 doctest failed
  • Almost all of these come from brial:
          File "/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/IPYTHON/sage-9.1.rc1/local/lib/python3.7/site-packages/brial/gbrefs.py", line 9, in <module>
            import imp
        ...
        DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses
    
  • It seems that jedi needs parso as a prerequisite, and that causes the failure in cmdline.py.
  • The error in misc/package.py looks like another one from sorting dictionaries, and so it should be dealt with at #29042:
    Failed example:
        installed_packages()  # optional - build
    Expected:
        {...'alabaster': ...'pynac': ...}
    

pynac appeared before alabaster in the actual output, unfortunately.

comment:34 Changed 21 months ago by git

  • Commit changed from 31bb55c0494c0538624c37dfb842f74f079da198 to cd347e1e37e9c8aadc2d46af83ba7caa0d8e755e

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

cd347e1trac 28197: add new package parso

comment:35 in reply to: ↑ 33 Changed 21 months ago by arojas

Replying to jhpalmieri:

Here is the preparser patch from comment:16 (although without the lines changing sys.getrecursionlimit()). When merged with #29042, I see these doctest failures:

Uhm, for some reason I haven't been getting emails from this ticket (and many others). The brial issues are fixed in 1.2.6, sage's package seems to be quite outdated (latest version is 1.2.8)

When I ported the preparser, I removed quite a few tests from SagePromptTransformer(), those should probably be brought back somewhere.

comment:36 Changed 21 months ago by jhpalmieri

  • Dependencies changed from #28190 #29042 #29428 to #28190 #29042 #29428 #29658

BRiAl update at #29658

comment:37 Changed 20 months ago by arojas

  • Dependencies changed from #28190 #29042 #29428 #29658 to #28190 #29042 #29428 #29658 #29774

comment:38 Changed 20 months ago by arojas

Opened #29774 to deal with the multiple deprecation warnings thrown by ipython with jedi>=0.16

comment:39 Changed 20 months ago by gh-kliem

This seems to basically work now. I'm testing it on top of #29042 now and for example with ubuntu eoan I'm getting only one new failure:

https://github.com/kliem/sage-test-27122/runs/742530683

File "src/sage/rings/qqbar.py", line 8096, in sage.rings.qqbar.ANBinaryExpr.exactify
Failed example:
    import sys; sys.getrecursionlimit()
Expected:
    1000
Got:
    3000
**********************************************************************
File "src/sage/rings/qqbar.py", line 8102, in sage.rings.qqbar.ANBinaryExpr.exactify
Failed example:
    sys.getrecursionlimit()
Expected:
    1000
Got:
    3000

I will report further results, once the tests are done.

comment:40 Changed 20 months ago by mkoeppe

  • Description modified (diff)

comment:41 Changed 20 months ago by mkoeppe

  • Description modified (diff)

comment:42 Changed 20 months ago by gh-kliem

Except for this qqbar test, all tests pass on my ubuntu bionic (testing this ticket on top of #29042).

As noted already tab completion is broken and #29774 fixes this.

comment:43 Changed 20 months ago by jhpalmieri

Is this coming from jedi/app/__init__.py, the line

sys.setrecursionlimit(3000)

?

I don't think the actual limit is important, just that it doesn't change, so I propose the following:

  • src/sage/rings/qqbar.py

    diff --git a/src/sage/rings/qqbar.py b/src/sage/rings/qqbar.py
    index 7fc0cb9917..f38195cdcc 100644
    a b class ANBinaryExpr(ANDescr): 
    80938093        do this by increasing the recursion level at each step and
    80948094        decrease it before we return::
    80958095
    8096             sage: import sys; sys.getrecursionlimit()
    8097             1000
     8096            sage: import sys
     8097            sage: old_limit = sys.getrecursionlimit()
    80988098            sage: s = SymmetricFunctions(QQ).schur()
    80998099            sage: a=s([3,2]).expand(8)(flatten([[QQbar.zeta(3)^d for d in range(3)], [QQbar.zeta(5)^d for d in range(5)]]))
    81008100            sage: a.exactify(); a # long time
    81018101            0
    8102             sage: sys.getrecursionlimit()
    8103             1000
     8102            sage: sys.getrecursionlimit() == old_limit
     8103            True
    81048104
    81058105        """
    81068106        import sys

comment:44 Changed 20 months ago by jhpalmieri

I ran into a problem building Sage because it tried to build backcall before pip was installed. So here is a new branch. (It doesn't include the proposed change from comment:43).

comment:45 Changed 20 months ago by git

  • Commit changed from cd347e1e37e9c8aadc2d46af83ba7caa0d8e755e to a9594b5951a0244d526da156eb461f0cd82a753c

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

cb86032trac 28197: IPython 7.13.0, with new required packages jedi, backcall
bbfbc4ftrac 28197: fix preparser for new IPython: patch taken from
61157b8trac 28197: add new package parso
a9594b5trac 28197: add pip as dependency for backcall, jedi, parso

comment:46 Changed 20 months ago by jhpalmieri

On OS X, with this + #29042, the qqbar test is the only one that fails for me.

comment:47 Changed 20 months ago by arojas

  • Cc changed from embray, jdemeyer, slelievre, arojas, fbissey, gh-timokau to embray, jdemeyer, slelievre, fbissey, gh-timokau, arojas

comment:48 Changed 20 months ago by mkoeppe

PYTHON_TOOLKIT should probably be PYTHON_TOOLCHAIN

comment:49 Changed 20 months ago by git

  • Commit changed from a9594b5951a0244d526da156eb461f0cd82a753c to 7f735773199643862f29238aee5790404e570630

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

7f73577trac 28197: add pip as dependency for backcall, jedi, parso

comment:50 Changed 20 months ago by jhpalmieri

Thanks, fixed.

comment:51 follow-up: Changed 20 months ago by mkoeppe

What is missing on this ticket?

comment:52 Changed 20 months ago by mkoeppe

  • Priority changed from major to critical

comment:53 Changed 20 months ago by jhpalmieri

In my opinion, the only thing missing is how to resolve the qqbar doctest failure. I proposed a solution in comment:43, but I would like some feedback before adding it to the branch. Maybe I'm missing something, or maybe there is a better solution.

comment:54 Changed 20 months ago by gh-kliem

The test tests two things, if I see this correctly:

  • Changing the recursion limit in the method allows for
    a=s([3,2]).expand(8)(flatten([[QQbar.zeta(3)^d for d in range(3)], [QQbar.zeta(5)^d for d in range(5)]]))
    
    to be exactified.
  • Also the recursion limit should be the same after the method was called.

If the initial recursion limit is already 3000, the test doesn't work anymore. As a can be exactified without modifying the recursion limit in the method exactify (I checked). So the test passes with the solution in comment:43, but it is almost meaningless.

I see two solutions for this:

  • Replace a by
    a=s([3,2]).expand(10)(flatten([[QQbar.zeta(3)^d for d in range(3)], [QQbar.zeta(7)^d for d in range(7)]]))
    
    This value cannot be exactified with recursion limit 3000, but it works fine once the method increases the recursion limit. However the doctest takes a bit longer.
  • We could modify the recursion limit before and after the doctest. This is faster. However, we rely on the same machinery as the method.

The advantage is that this really is a long time solution. If with some new update the default recursion limit is increased again, we can still see that the method works correctly.

comment:55 Changed 19 months ago by jhpalmieri

With the longer version (your first proposal), a.exactify() takes over 10 seconds on my computer, which is longer than we usually aim for with tests. So I would prefer your second proposal.

comment:56 in reply to: ↑ 51 Changed 19 months ago by arojas

Replying to mkoeppe:

What is missing on this ticket?

The removed tests from the old SagePromptTransformer?() need to be restored

comment:57 Changed 19 months ago by gh-kliem

  • Branch changed from public/ticket/28197 to public/ticket/28197-reb
  • Commit changed from 7f735773199643862f29238aee5790404e570630 to 41e63557a26f754714950bff11c546606d2f76ef

New commits:

f832bbetrac 28197: IPython 7.13.0, with new required packages jedi, backcall
e3ebfc1trac 28197: fix preparser for new IPython: patch taken from
dc6e1d8trac 28197: add new package parso
7ee3f3dtrac 28197: add pip as dependency for backcall, jedi, parso
41e6355lower the recursion limit for exactification test

comment:58 Changed 19 months ago by git

  • Commit changed from 41e63557a26f754714950bff11c546606d2f76ef to 114d322aed47382852f186e6d60eb98264118be5

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

114d322readd prompt transformer doc tests

comment:59 Changed 19 months ago by gh-kliem

  • Authors set to Jonathan Kliem, ​John H. Palmieri, Antonio Rojas
  • Status changed from new to needs_review

As for the authors, I kind of guessed. Is this correct? Am I missing something?

Also I don't really know how we do it with order of authors. I went alphabetically. However, I just fixed doctests.

comment:60 follow-up: Changed 19 months ago by arojas

diff --git a/src/sage/rings/qqbar.py b/src/sage/rings/qqbar.py
index 7fa7f8d..fd4b3bf 100644
--- a/src/sage/rings/qqbar.py
+++ b/src/sage/rings/qqbar.py
@@ -8091,9 +8091,16 @@ class ANBinaryExpr(ANDescr):
 
         We check to make sure that this method still works even. We
         do this by increasing the recursion level at each step and
-        decrease it before we return::
+        decrease it before we return.
+        We lower the recursion limit for this test to allow
+        a test in reasonable time::
 
-            sage: import sys; sys.getrecursionlimit()
+            sage: import sys
+            sage: old_recursion_limit = sys.getrecursionlimit()
+            sage: sys.setrecursionlimit(1000)
+
+            sage: old_recursion_limit = sys.getrecursionlimit()
+            sage: sys.getrecursionlimit()
             1000
             sage: s = SymmetricFunctions(QQ).schur()
             sage: a=s([3,2]).expand(8)(flatten([[QQbar.zeta(3)^d for d in range(3)], [QQbar.zeta(5)^d for d in range(5)]]))

The second old_recursion_limit = sys.getrecursionlimit() here doesn't look right. This will always set the recursion limit to 1000 at the end regardless of the original value

comment:61 Changed 19 months ago by git

  • Commit changed from 114d322aed47382852f186e6d60eb98264118be5 to 751864be3ff6a170ae7a9071b3bd586211697bd9

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

751864bfixed mistake

comment:62 in reply to: ↑ 60 Changed 19 months ago by gh-kliem

Thanks for catching this.

Replying to arojas:

diff --git a/src/sage/rings/qqbar.py b/src/sage/rings/qqbar.py
index 7fa7f8d..fd4b3bf 100644
--- a/src/sage/rings/qqbar.py
+++ b/src/sage/rings/qqbar.py
@@ -8091,9 +8091,16 @@ class ANBinaryExpr(ANDescr):
 
         We check to make sure that this method still works even. We
         do this by increasing the recursion level at each step and
-        decrease it before we return::
+        decrease it before we return.
+        We lower the recursion limit for this test to allow
+        a test in reasonable time::
 
-            sage: import sys; sys.getrecursionlimit()
+            sage: import sys
+            sage: old_recursion_limit = sys.getrecursionlimit()
+            sage: sys.setrecursionlimit(1000)
+
+            sage: old_recursion_limit = sys.getrecursionlimit()
+            sage: sys.getrecursionlimit()
             1000
             sage: s = SymmetricFunctions(QQ).schur()
             sage: a=s([3,2]).expand(8)(flatten([[QQbar.zeta(3)^d for d in range(3)], [QQbar.zeta(5)^d for d in range(5)]]))

The second old_recursion_limit = sys.getrecursionlimit() here doesn't look right. This will always set the recursion limit to 1000 at the end regardless of the original value

comment:63 Changed 19 months ago by jhpalmieri

Are all of the issues taken care of? Tests pass for me on OS X (after merging #29042).

comment:64 Changed 19 months ago by slelievre

  • Authors changed from Jonathan Kliem, ​John H. Palmieri, Antonio Rojas to Jonathan Kliem, ​John Palmieri, Antonio Rojas
  • Keywords upgrade added

(Fixing John Palmieri's name in "Authors" field to match its spelling in other tickets.)

comment:65 Changed 19 months ago by chapoton

So, can we move on here ? Patchbots are helpless to check that nothing is broken. Does the gitlab CI help in this case ?

comment:66 Changed 19 months ago by gh-kliem

I started a test run pulling this ticket and #29851 in the newest develop (#29851 fixes the workflow for debian bullseye and debian sid, so this seems reasonable for a good testing experience, it just allows reinstalling python for them, so really nothing to do with our ticket).

Results will be available here https://github.com/kliem/sage/pull/16/checks.

comment:67 Changed 19 months ago by jhpalmieri

This still works for me on OS X, after merging with the latest develop branch. I'll switch to positive review in a few days, naming some collection of reviewers unless (a) someone objects or (b) someone beats me to it.

comment:68 Changed 19 months ago by mkoeppe

  • Reviewers set to John Palmieri, Matthias Koeppe

I have also tested an earlier version successfully as part of #29864 (namespace packages). Let's get this into the next beta for wider testing.

comment:69 Changed 19 months ago by gh-kliem

As for the tests, there is no obvious failure. So at least installing seems to work everywhere. I didn't test any generated docker images to see how it actually looks though.

comment:70 Changed 19 months ago by jhpalmieri

  • Status changed from needs_review to positive_review

I'm carrying out my threat to set a positive review ;)

Dear all: feel free to add yourself as a reviewer or an author (or both), as you see fit.

comment:71 follow-up: Changed 19 months ago by jhpalmieri

For a future ticket, we could also upgrade to 7.16.1. I just tried, and it builds and passes tests on my OS X box. I want to get this ticket in as it stands, though.

comment:72 in reply to: ↑ 71 Changed 19 months ago by fbissey

Replying to jhpalmieri:

For a future ticket, we could also upgrade to 7.16.1. I just tried, and it builds and passes tests on my OS X box. I want to get this ticket in as it stands, though.

For the record, I am at 7.16.1 in sage-on-gentoo as well. Merging as it it now is the most important bit.

comment:73 Changed 19 months ago by isuruf

Looks like merge failed

comment:74 Changed 19 months ago by git

  • Commit changed from 751864be3ff6a170ae7a9071b3bd586211697bd9 to e21a7b0eec15aac948ff62e0379207471cdc7de8
  • Status changed from positive_review to needs_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

e21a7b0Merge tag '9.2.beta5' into t/28197/public/ticket/28197-reb

comment:75 Changed 19 months ago by mkoeppe

  • Status changed from needs_review to positive_review

comment:76 Changed 18 months ago by mkoeppe

  • Dependencies #28190 #29042 #29428 #29658 #29774 deleted

comment:77 Changed 18 months ago by vbraun

  • Branch changed from public/ticket/28197-reb to e21a7b0eec15aac948ff62e0379207471cdc7de8
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:78 Changed 18 months ago by charpent

  • Commit e21a7b0eec15aac948ff62e0379207471cdc7de8 deleted

This update broke sage-shell-mode support for Sage session in emacs.

comment:79 Changed 17 months ago by gh-mwageringel

Preparsing multi-line strings also seems to be broken by this upgrade. See #30417.

comment:80 Changed 15 months ago by mkoeppe

  • Authors changed from Jonathan Kliem, ​John Palmieri, Antonio Rojas to Jonathan Kliem, John Palmieri, Antonio Rojas

comment:81 Changed 14 months ago by egourgoulhon

See #30928 for a possible follow-up.

Note: See TracTickets for help on using tickets.