Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to Python 3.7.x #25680

Closed
jdemeyer opened this issue Jun 28, 2018 · 217 comments
Closed

Upgrade to Python 3.7.x #25680

jdemeyer opened this issue Jun 28, 2018 · 217 comments

Comments

@jdemeyer
Copy link
Contributor

Tarball: https://www.python.org/ftp/python/3.7.3/Python-3.7.3.tar.xz

CC: @fchapoton @embray @kiwifb @mkoeppe @slel

Component: packages: standard

Keywords: upgrade

Author: Jeroen Demeyer

Branch: 5a09cf1

Reviewer: Vincent Klein, Erik Bray, Frédéric Chapoton

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

@jdemeyer
Copy link
Contributor Author

Branch: u/jdemeyer/upgrade_to_python_3_7_0

@jdemeyer
Copy link
Contributor Author

New commits:

2c67605Upgrade to Python 3.7.0

@jdemeyer
Copy link
Contributor Author

Commit: 2c67605

@jdemeyer
Copy link
Contributor Author

Author: Jeroen Demeyer

@embray
Copy link
Contributor

embray commented Jun 28, 2018

comment:3

I don't think we should be in any hurry on this. All the last ~year's work of Python 3 porting has been targeting Python 3.6, and I think I'd rather stick with that before risking throwing everything out of whack again.

Between Python 3.6 and 3.7 I suspect the differences, if any, will be minor. But I'd rather try to stabilize on 3.6 first (to which we're quite close), then deal with those differences, rather than change Python versions again. Also I don't know that Python 3.7 has been ported to Cygwin yet (although it does contain a number of useful fixes for Cygwin, I do know).

That said, if you know any specific changes in Python 3.7 (other than the aforementioned Cygwin fixes) that would actively make our porting effort easier I'd consider it...

@fchapoton
Copy link
Contributor

comment:5

Jeroen has said elsewhere that islice will accept Sage integers instead of just ints.

I am again becoming tired and frustated by the slow pace of progress towards py3. And by the large amount of difficult work that remains..

@embray
Copy link
Contributor

embray commented Jun 28, 2018

comment:6

Ah, I remember making an issue about that, but I don't think I followed what the resolution was.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 28, 2018

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

1bfcdc8Upgrade to Python 3.7.0

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 28, 2018

Changed commit from 2c67605 to 1bfcdc8

@embray
Copy link
Contributor

embray commented Jun 28, 2018

comment:8

Well there's this python/cpython#1918 But unless there are other fixes I'm not seeing it's annoying that this was only fixed for islice, where I believe there is other code affected by the same issue (e.g. range). Also we've already worked aroudn the majority of those cases (sigh...)

@embray
Copy link
Contributor

embray commented Jun 28, 2018

comment:9

Replying to @fchapoton:

I am again becoming tired and frustated by the slow pace of progress towards py3. And by the large amount of difficult work that remains..

Unfortunately I'm not convinced that upgrading Python yet again is going to make that go any faster. In any case I don't actually have that much "difficult" work remaining. Just dozens of little issues. My Python 3 branch has < 600 modules failing, and most of it appears to minor issues (at one point I had it down to ~400 and I don't know why it ballooned up again, but one or two small changes in just the right places can do that with Sage...)

Also I'll add if you're not using a patched pynac you're going to have lots, lots more "serious" looking issues. We still need to get a pynac upgrade in (I've been manually installing pynac with my Python 3 fixes every time I re-build sage).

@jdemeyer
Copy link
Contributor Author

comment:10

Replying to @embray:

That said, if you know any specific changes in Python 3.7 (other than the aforementioned Cygwin fixes) that would actively make our porting effort easier I'd consider it...

#25391 is a pretty serious issue which is fixed by a Python upgrade (probably also by upgrading to 3.6.6 but I haven't tested that).

@jdemeyer
Copy link
Contributor Author

comment:11

Replying to @embray:

I don't think we should be in any hurry on this. All the last ~year's work of Python 3 porting has been targeting Python 3.6, and I think I'd rather stick with that before risking throwing everything out of whack again.

First of all, I do not think that this 3.6 -> 3.7 will throw everything out of whack.

However, regardless of that, we'll have to upgrade sooner or later. I don't see the point of having a perfectly working Sage on an outdated Python version. And for the many issues that we still have to fix, it would be better to make sure that they work on Python 3.7 from the start.

@fchapoton
Copy link
Contributor

comment:12

I still think we have very hard remaining problems, among which

  • sorting issues in graphs
  • hashing and comparisons, especially for groups
  • coercion framework have broken parts

@vinklein
Copy link
Mannequin

vinklein mannequin commented Jun 29, 2018

Attachment: python3-3.7.0.log

@vinklein
Copy link
Mannequin

vinklein mannequin commented Jun 29, 2018

comment:14

I get the following error

$ ./configure --with-python=3
...
$ make build
...
[python3-3.7.0] Testing importing of various modules...
[python3-3.7.0] Traceback (most recent call last):
[python3-3.7.0]   File "<string>", line 1, in <module>
[python3-3.7.0]   File "/home/vklein/odk/sage/local/var/tmp/sage/build/python3-3.7.0/src/Lib/ctypes/__init__.py", line 7, in <module>
[python3-3.7.0]     from _ctypes import Union, Structure, Array
[python3-3.7.0] ModuleNotFoundError: No module named '_ctypes'
[python3-3.7.0] ctypes module failed to import
[python3-3.7.0] math module imported OK
[python3-3.7.0] hashlib module imported OK
[python3-3.7.0] crypt module imported OK
[python3-3.7.0] readline module imported OK
[python3-3.7.0] socket module imported OK
[python3-3.7.0] Error: One or more modules failed to import.
...

full python3.7 install log in attachement

@kiwifb
Copy link
Member

kiwifb commented Jun 29, 2018

comment:15

Failure to build _ctypes usually means a problem with ffi (sometimes called libffi) may be there is minimum version requirement?

@vinklein
Copy link
Mannequin

vinklein mannequin commented Jun 29, 2018

comment:16

Indeed i don't get the error after installing libffi-dev. Thanks !

Note : sage make build works fine in python 3.6 without that.

@vinklein
Copy link
Mannequin

vinklein mannequin commented Jun 29, 2018

comment:17

next error during the build is :

Installing collected packages: ipykernel
  Running setup.py install for ipykernel: started
    Running command /home/vklein/odk/sage/local/bin/python3 -u -c "import setuptools, tokenize;__file__='/tmp/pip-qoxsao1r-build/setup.py';f=getattr(tokenize
, 'open', open)(__file__);code=f.read().replace('\r\n', '\n');f.close();exec(compile(code, __file__, 'exec'))" --no-user-cfg install --record /tmp/pip-vc23lh
pc-record/install-record.txt --single-version-externally-managed --compile
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "/tmp/pip-qoxsao1r-build/setup.py", line 91, in <module>
        from ipykernel.kernelspec import write_kernel_spec, make_ipkernel_cmd, KERNEL_NAME
...
      File "/home/vklein/odk/sage/local/lib/python3.7/site-packages/pexpect/spawnbase.py", line 224
        def expect(self, pattern, timeout=-1, searchwindowsize=-1, async=False):
                                                                       ^
    SyntaxError: invalid syntax
    Running setup.py install for ipykernel: finished with status 'error'
Cleaning up...

Issue reference : pypa/pipenv#956

@kiwifb
Copy link
Member

kiwifb commented Jun 29, 2018

comment:18

To be honest we haven't updated pexpect in vanilla sage for a while. It may be a good idea to do that - in a separate ticket.

@vinklein
Copy link
Mannequin

vinklein mannequin commented Jun 29, 2018

comment:19

Do we need to update pexpect or to patch pipenv ?

@jdemeyer
Copy link
Contributor Author

comment:20

No, pexpect.

@kiwifb
Copy link
Member

kiwifb commented Jun 29, 2018

comment:21

I am at pexpect-4.2.1 in sage-on-gentoo (without any patches) and sage is at 4.1.0 + some patches.

@jdemeyer
Copy link
Contributor Author

comment:173

I'm getting docbuild failures due to

WARNING: cannot copy static file OSError(40, 'Too many levels of symbolic links')

@jhpalmieri
Copy link
Member

comment:174

I don't see this warning. You don't have that stupid mathjax recursive link, do you? Oh, make ptest-python3 passes for me.

@jdemeyer
Copy link
Contributor Author

comment:175

I don't know what was wrong with the docs. I did make doc-clean and then it worked.

@embray
Copy link
Contributor

embray commented Mar 27, 2019

comment:176

I am having a bad problem when running the tests where every test outputs something like:

Exception ignored in: <function _TemporaryFileCloser.__del__ at 0x6ffff4d92f0>
Traceback (most recent call last):
  File "/home/embray/src/sagemath/sage-python3/local/lib/python3.7/tempfile.py", line 448, in __del__
    self.close()
  File "/home/embray/src/sagemath/sage-python3/local/lib/python3.7/tempfile.py", line 444, in close
    unlink(self.name)
FileNotFoundError: [Errno 2] No such file or directory: '/tmp/tmpw4zt6tfq'

even if the test succeeds. I think this may be a bug (big shock) in the tempfile module itself. I believe it might even be one that I already have a patch for (I seem to recall fixing some issues in tempfile back when I was trying to get all of the Python test suite to pass on Cygwin) but I will need to look into that before saying anything more.

@embray
Copy link
Contributor

embray commented Mar 27, 2019

comment:177

(Obviously the bug itself is relatively harmless: It's just running a __del__ method that unlinks a file but doesn't ignore errors that occur if the file was already deleted. It's more a nuisance than anything).

@embray
Copy link
Contributor

embray commented Mar 27, 2019

comment:178

I see; this is a recurrence of the problem I originally described here: #25107 comment:14

It's resurfaced, because the tempfile module has been refactored, now in such a way that merely setting the _TemporaryFileWrapper.deleted attribute no longer has any effect whatsoever (it should really probably be a property which delegates to the new _TemporaryFileCloser object.

I think this may be a broader issue with NamedTemporaryFile that is not unique just to Cygwin: If you create a NamedTemporaryFile(delete=True) in a parent process, then add it as an attribute on a Process subclass, when the worker process exits the file is deleted. But then when the temporary file is closed on the parent side it tries to re-delete the already deleted file. I think more generally it would be best if ENOENT errors were just ignored in this case.

@embray
Copy link
Contributor

embray commented Mar 27, 2019

comment:179

If you would consider including this commit, or something like it, that would resolve the problem I'm having with unhandled exceptions in _TemporaryFileCloser.__del__ .

With this fix, I can see more clearly that there are two test failures remaining on Cygwin:

sage -t --long src/sage_setup/docbuild/__init__.py  # 1 doctest failed
sage -t --long src/sage/manifolds/differentiable/vectorfield.py  # Killed due to segmentation fault

The first one is merely a manifestation of #27514. Still need to rethink exactly how that test is implemented.

The second is more disconcerting, being a segfault (but at least repeatable). I will look into it next.

@embray
Copy link
Contributor

embray commented Mar 27, 2019

Changed reviewer from Vincent Klein to Vincent Klein, Erik Bray

@embray
Copy link
Contributor

embray commented Mar 27, 2019

comment:181

I think the segfault might be related to openblas issues that I've already fixed, but may not be incorporated in my current build (since I started building this Python 3 branch some time ago, possibly before all the relevant openblas fixes were incorporated).

@fchapoton
Copy link
Contributor

Changed dependencies from #27523 to none

@fchapoton
Copy link
Contributor

Changed reviewer from Vincent Klein, Erik Bray to Vincent Klein, Erik Bray, Frédéric Chapoton

@fchapoton
Copy link
Contributor

comment:183

I am setting this to positive now, so that we can start again making progress on #27519. We can always fix details in later tickets.

@vbraun
Copy link
Member

vbraun commented Mar 31, 2019

Changed branch from u/jdemeyer/upgrade_to_python_3_7_0 to 5a09cf1

@embray
Copy link
Contributor

embray commented Apr 1, 2019

comment:185

Replying to @fchapoton:

I am setting this to positive now, so that we can start again making progress on #27519. We can always fix details in later tickets.

Why would you do that? I explicitly pointed out that something was broken and proposed a fix, but that fix wasn't applied yet. Doing this "now" has no direct impact on making progress on #27519. You're just rushing things for no reason and leaving them broken.

@embray
Copy link
Contributor

embray commented Apr 1, 2019

Changed commit from 5a09cf1 to none

@embray
Copy link
Contributor

embray commented Apr 1, 2019

comment:186

Replying to @embray:

I think the segfault might be related to openblas issues that I've already fixed, but may not be incorporated in my current build (since I started building this Python 3 branch some time ago, possibly before all the relevant openblas fixes were incorporated).

I meant to follow up here. The segfault was nothing to do with Python 3 (fortunately) and is fixed by #27565 . Not exactly sure why I wasn't seeing that bug before--it's possible I was building OpenBLAS out of its recent develop branch which already had that fixed.

@kiwifb
Copy link
Member

kiwifb commented Apr 16, 2019

comment:187

Do we have a follow up on that ticket anywhere? In sage-on-gentoo I have a segfault when building the documentation with python 3.7.3 (but not 2.7 or 3.6). Am I alone with that issue?

python3.7: /dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_7/build/cythonized/sage/matrix/matrix1.c:14275: __pyx_pf_4sage_6matrix_7matrix1_6Matrix_60matrix_from_columns: Assertion `PyList_Check(__pyx_v_columns)' failed.

@jdemeyer
Copy link
Contributor Author

comment:188

Interesting. You're building with assertions enabled.

@jdemeyer
Copy link
Contributor Author

comment:189

Can you attach or send me that matrix1.c file? In my version of that file, I don't find such an assertion.

@kiwifb
Copy link
Member

kiwifb commented Apr 17, 2019

comment:190

Attachment: matrix1.c.bz2.gz

Attached as requested (compressed).

@jdemeyer
Copy link
Contributor Author

comment:191

Thanks, that clears things up. Follow-up ticket at #27688.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants