Opened 3 years ago

Closed 2 years ago

Last modified 21 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 3 years ago by chapoton

  • Description modified (diff)

comment:10 Changed 3 years ago by fbissey

  • Cc fbissey added

comment:11 Changed 3 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 3 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 3 years ago by gh-timokau

  • Cc gh-timokau added

comment:14 Changed 3 years ago by jhpalmieri

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

comment:15 Changed 3 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 3 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 3 years ago by embray

  • Milestone changed from sage-8.9 to sage-9.1

Ticket retargeted after milestone closed

comment:18 Changed 3 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 3 years ago by chapoton

a preparation step at #28948

comment:20 Changed 3 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 2 years 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 2 years 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 2 years 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 2 years 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 2 years 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 2 years ago by fbissey

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

comment:27 Changed 2 years ago by mkoeppe

  • Milestone changed from sage-9.1 to sage-9.2

comment:28 Changed 2 years ago by arojas

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

comment:29 Changed 2 years 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 2 years 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 2 years ago by jhpalmieri

  • Description modified (diff)

comment:32 Changed 2 years 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 2 years 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 2 years 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 2 years 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 2 years ago by jhpalmieri

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

BRiAl update at #29658

comment:37 Changed 2 years ago by arojas

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

comment:38 Changed 2 years ago by arojas

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

comment:39 Changed 2 years 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 2 years ago by mkoeppe

  • Description modified (diff)

comment:41 Changed 2 years ago by mkoeppe

  • Description modified (diff)

comment:42 Changed 2 years 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 2 years 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 2 years 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 2 years 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 2 years ago by jhpalmieri

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

comment:47 Changed 2 years ago by arojas

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

comment:48 Changed 2 years ago by mkoeppe

PYTHON_TOOLKIT should probably be PYTHON_TOOLCHAIN

comment:49 Changed 2 years 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 2 years ago by jhpalmieri

Thanks, fixed.

comment:51 follow-up: Changed 2 years ago by mkoeppe

What is missing on this ticket?

comment:52 Changed 2 years ago by mkoeppe

  • Priority changed from major to critical

comment:53 Changed 2 years 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 2 years 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 2 years 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 2 years 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 2 years 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 2 years 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 2 years 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 2 years 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 2 years 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 2 years 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 2 years ago by jhpalmieri

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

comment:64 Changed 2 years 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 2 years 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 2 years 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 2 years 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 2 years 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 2 years 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 2 years 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 2 years 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 2 years 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 2 years ago by isuruf

Looks like merge failed

comment:74 Changed 2 years 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 2 years ago by mkoeppe

  • Status changed from needs_review to positive_review

comment:76 Changed 2 years ago by mkoeppe

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

comment:77 Changed 2 years 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 2 years ago by charpent

  • Commit e21a7b0eec15aac948ff62e0379207471cdc7de8 deleted

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

comment:79 Changed 2 years ago by gh-mwageringel

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

comment:80 Changed 22 months ago by mkoeppe

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

comment:81 Changed 21 months ago by egourgoulhon

See #30928 for a possible follow-up.

Note: See TracTickets for help on using tickets.