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

For Sage 9.2: Remove python 2 support from sagelib #28000

Closed
embray opened this issue Jun 16, 2019 · 50 comments
Closed

For Sage 9.2: Remove python 2 support from sagelib #28000

embray opened this issue Jun 16, 2019 · 50 comments

Comments

@embray
Copy link
Contributor

embray commented Jun 16, 2019

Python 3 support (#15530) was completed for Sage 9.0 (thanks in particular to the leadership of Frédéric Chapoton). Official support for Python 2.7 by the CPython core developers is ending January 1, 2020.

To enable major package upgrades and other changes that require Python 3.x (#29141),
this ticket removes Python 2 support in Sage for Sage 9.2.

Depends on #26403
Depends on #28660

CC: @timokau @fchapoton @dimpase

Component: python3

Keywords: days101

Author: Erik Bray

Branch: f368e32

Reviewer: Dima Pasechnik

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

@embray

This comment has been minimized.

@jhpalmieri
Copy link
Member

comment:3

What needs to be done for this ticket?

@embray
Copy link
Contributor Author

embray commented Jun 17, 2019

comment:4

Go through and remove all hacks related to supporting Python 2. It will be interesting to see how big a change it takes even now. It's rather early to go through and do that in earnest, but it would be fun to just try and see what it takes.

@slel
Copy link
Member

slel commented Jun 19, 2019

Changed keywords from none to days101

@jhpalmieri
Copy link
Member

comment:6

One example: #27696 (remove the doctesting hack for "long", which just converts it to "int" when using Python 3.

@slel
Copy link
Member

slel commented Jun 28, 2019

comment:7

Also, this ticket can be set as a dependency for other tickets that depend on the
removal of Python 2 support. Many dependency upgrades will depend on that.

Might it make sense to migrate the whole Jupyter part to Python3-only versions
before completely removing Python 3 support? Even when building Sage for
Python 2, we ship Python 3 too anyway.

@embray
Copy link
Contributor Author

embray commented Jul 1, 2019

comment:8

I made some progress at days101 removing all dependency on the six module for Python 2/3 support and that alone was a HUGE change (and not even quite finished yet). Will post a branch once I'm done with that.

@embray
Copy link
Contributor Author

embray commented Aug 18, 2019

comment:9

Here's some of my progress on this so far: Probably one of the biggest, most pervasive steps, being complete removal of dependency on six.

Of course, this step is optional--we can keep using six while otherwise dropping support for Python 2. But this makes it quite explicit.

It's pretty hard to avoid making a patch bomb of this. It's not all bad though--I started this months ago, and then once I got it finished I rebased on current develop and only had about a dozen minor merge conflicts.

@embray
Copy link
Contributor Author

embray commented Aug 18, 2019

Branch: u/embray/python3/ticket-28000

@embray
Copy link
Contributor Author

embray commented Aug 18, 2019

Author: Erik Bray

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 18, 2019

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

8f3cacbTrac #28000: Remove sagelib dependency on six

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 18, 2019

Commit: 8f3cacb

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 18, 2019

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

736c7a2One Python 2-specific block that did not use six.PY2.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 18, 2019

Changed commit from 8f3cacb to 736c7a2

@jhpalmieri
Copy link
Member

comment:12

Other tasks:

  • remove or revise the doctests labeled # py2. Ideally, all such tests should be evaluated on a case-by-case basis, some should just be removed, but perhaps some should be modified to work with Python 3.
  • With Python 3 build, do not install python2 #28426: with a Python 3 build, do not build Python 2.
  • remove some packages which are only necessary with Python 2: typing (py3: typing should not be installed in Python 3 #28499) is the only one I know of currently, but I wouldn't be surprised if there were others.
  • remove the legacy Sage notebook — at least, I don't think it works with Python 3 now, and I don't think anyone is working on it. This means we should deprecate it ASAP. (Make the sage notebook optional #25837)

@embray
Copy link
Contributor Author

embray commented Sep 18, 2019

comment:13

My branch for this has some other in-progress changes that it seems I haven't pushed yet. I'll do so next time I rebase this branch.

@embray
Copy link
Contributor Author

embray commented Sep 18, 2019

comment:14

Oh right, one of the changes I started was to remove unnecessary from __future__ imports, but it seems until we have #26403, to my slight surprise, some of them are still needed in Cython modules, e.g. to have the print() function by default.

@embray
Copy link
Contributor Author

embray commented Sep 18, 2019

Dependencies: #26403

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 18, 2019

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

d2c4427Trac #28000: Remove sagelib dependency on six
aab5d3bOne Python 2-specific block that did not use six.PY2.
50b8951Trac #28000: Get rid of most `__nonzero__` references and/or replace them with __bool__

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 18, 2019

Changed commit from 736c7a2 to 50b8951

@embray
Copy link
Contributor Author

embray commented Sep 18, 2019

comment:17

Rebased, but no guarantee there are no new bugs or new uses of six I didn't miss. I didn't check yet.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 18, 2019

Changed commit from 50b8951 to 1f6a41d

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 18, 2019

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

972f431Trac #28000: Remove sagelib dependency on six
4c35164One Python 2-specific block that did not use six.PY2.
1f6a41dTrac #28000: Get rid of most `__nonzero__` references and/or replace them with __bool__

@embray
Copy link
Contributor Author

embray commented Sep 18, 2019

comment:19

Updated again after removing two new uses of six that have since appeared.

@slel
Copy link
Member

slel commented Oct 27, 2019

Changed dependencies from #26403 to #26403, #28660

@jhpalmieri
Copy link
Member

comment:21

Do we remove all doctests tagged # py2 here or on a separate ticket?

@embray
Copy link
Contributor Author

embray commented Nov 19, 2019

comment:22

You could do it here, or a separate ticket (and make it a dependency of this ticket). It's going to be a patch bomb no matter what, but I don't see the harm in trying to split it up wehre possible.

@mkoeppe

This comment has been minimized.

@mkoeppe mkoeppe added this to the sage-9.2 milestone May 9, 2020
@mkoeppe
Copy link
Contributor

mkoeppe commented May 9, 2020

comment:27

Needs rebasing!

@mkoeppe mkoeppe changed the title Remove python 2 support For Sage 9.2: Remove python 2 support from sagelib May 9, 2020
@jhpalmieri
Copy link
Member

comment:29

Related: #27826

@jhpalmieri
Copy link
Member

@jhpalmieri
Copy link
Member

comment:31

Rebased. I haven't seen activity from embray for a while. Perhaps this is ready for review?


New commits:

f368e32trac 28000: rebase commits from Erik M. Bray :

@jhpalmieri
Copy link
Member

Changed commit from 0e37a95 to f368e32

@mkoeppe
Copy link
Contributor

mkoeppe commented May 19, 2020

comment:32

Let's see if this finds a reviewer

@dimpase
Copy link
Member

dimpase commented May 19, 2020

Reviewer: Dima Pasechnik

@dimpase
Copy link
Member

dimpase commented May 19, 2020

comment:33

testing.

@dimpase
Copy link
Member

dimpase commented May 20, 2020

comment:34

OK, this makes sense and appears to work. A long, but straightforward patch, also touching quite a bit of whitespace (which probably could have waited for another place, but OK).

Should removing Python2 spkg be done elsewhere?

@mkoeppe
Copy link
Contributor

mkoeppe commented May 20, 2020

comment:35

Replying to @dimpase:

Should removing Python2 spkg be done elsewhere?

Yes, see #29141 (Meta-ticket: Remove Python 2 support for Sage 9.2; upgrades enabled by the removal)

@vbraun
Copy link
Member

vbraun commented May 26, 2020

Changed branch from u/jhpalmieri/python3/ticket-28000 to f368e32

@timokau
Copy link
Contributor

timokau commented May 27, 2020

Changed commit from f368e32 to none

@timokau
Copy link
Contributor

timokau commented May 27, 2020

comment:37

\(^O^)/

Hopefully the divergence between sage-the-distribution and sage-on-distro will decrease now and packaging will become simpler again.

@embray
Copy link
Contributor Author

embray commented May 28, 2020

comment:38

Thanks for finally getting this ticket merged! Sorry I haven't had much time to work on it. As others have noted this does not take care of all tasks related to dropping Python 2 support--it was just me doing as much as I could in one place. Took hours!

@vbraun
Copy link
Member

vbraun commented May 28, 2020

comment:39

This breaks on Python 3.5, I've created #29753 for that

@jhpalmieri
Copy link
Member

comment:40

Does anyone know of any docker setups from tox.ini that have Python 3.5?

@dimpase
Copy link
Member

dimpase commented May 28, 2020

comment:41

Python 3.5 will be gone in Sept, why would wr spend time trying to support it?

@mwageringel
Copy link

comment:42

This accidentally broke the Macaulay2 interface. Follow up at #29759 with a trivial fix.

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

8 participants