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 Cython 0.29.1 #25292

Closed
jdemeyer opened this issue May 4, 2018 · 48 comments
Closed

Upgrade to Cython 0.29.1 #25292

jdemeyer opened this issue May 4, 2018 · 48 comments

Comments

@jdemeyer
Copy link

jdemeyer commented May 4, 2018

Tarball: https://files.pythonhosted.org/packages/f0/f8/7f406aac4c6919d5a4ce16509bbe059cd256e9ad94bae5ccac14094b7c51/Cython-0.29.1.tar.gz

Follow-up tickets:

Upstream: Fixed upstream, but not in a stable release.

CC: @kiwifb @embray @saraedum

Component: packages: standard

Author: Jeroen Demeyer

Branch/Commit: 4c3e314

Reviewer: Timo Kaufmann

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

@jdemeyer jdemeyer added this to the sage-8.3 milestone May 4, 2018
@videlec
Copy link
Contributor

videlec commented Aug 3, 2018

comment:1

update milestone 8.3 -> 8.4

@videlec videlec modified the milestones: sage-8.3, sage-8.4 Aug 3, 2018
@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link
Author

jdemeyer commented Oct 4, 2018

Author: Jeroen Demeyer

@jdemeyer
Copy link
Author

jdemeyer commented Oct 4, 2018

Dependencies: #26396

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link
Author

jdemeyer commented Oct 4, 2018

Branch: u/jdemeyer/upgrade_to_cython_0_29

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 4, 2018

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

3275637Explicitly set Cython language level to current Python version

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 4, 2018

Commit: 3275637

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link
Author

jdemeyer commented Oct 4, 2018

Changed dependencies from #26396 to #26396, #26402

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link
Author

Changed dependencies from #26396, #26402 to #26396

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 14, 2018

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

bcba4f1Upgrade to cypari2-1.3.1
f77de1dUpgrade to Cython 0.29

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 14, 2018

Changed commit from 3275637 to f77de1d

@Konrad127123
Copy link

comment:13

Attachment: cython-0.29.log

Running make distclean && make cython with SAGE_CHECK=yes, I'm reproducibly getting one test failing (see log).

(The two failing tests in #26450 don't seem to be an issue anymore in 0.29.)

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 23, 2018

Changed commit from f77de1d to 7ffa946

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 23, 2018

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

7ffa946Upgrade to Cython 0.29

@jdemeyer
Copy link
Author

Upstream: Fixed upstream, but not in a stable release.

@jdemeyer
Copy link
Author

comment:16

Replying to @Konrad127123:

Running make distclean && make cython with SAGE_CHECK=yes, I'm reproducibly getting one test failing (see log).

Fixed by cython/cython#2674

@jdemeyer
Copy link
Author

Changed dependencies from #26396 to none

@Konrad127123
Copy link

comment:18

Replying to @jdemeyer:

Replying to @Konrad127123:

Running make distclean && make cython with SAGE_CHECK=yes, I'm reproducibly getting one test failing (see log).

Fixed by cython/cython#2674

I can confirm that cython now builds with SAGE_CHECK=yes. I've not tested it apart from that.

@jdemeyer
Copy link
Author

comment:19

Can somebody review this please? There are follow-up tickets depending on this one.

@embray
Copy link
Contributor

embray commented Oct 31, 2018

comment:20

Does this mean Cython 0.29 will become a hard dependency of Sage? I don't mind that ofc, but maybe the tickets that depend on it can be done in such a way that checks this (e.g., checks the Cython version before enabling those features).

I'm going to give this a quick check on Cygwin, but I doubt there will be any problem, so positive_review from me if it that works.

@timokau
Copy link
Contributor

timokau commented Nov 26, 2018

comment:29

For what its worth, here is the status of cython on various distros and these are the distros that package sage. Debian Unstable hasn't upgraded yet. Arch and Nix have. Gentoo isn't listed for some reason.

@kiwifb
Copy link
Member

kiwifb commented Nov 26, 2018

comment:30

Replying to @timokau:

For what its worth, here is the status of cython on various distros and these are the distros that package sage. Debian Unstable hasn't upgraded yet. Arch and Nix have. Gentoo isn't listed for some reason.

Gentoo main tree, doesn't have it yet. But the sage-on-gentoo repo does have a package ready for when this ticket makes it regardless of the main tree.

@Konrad127123
Copy link

comment:31

Note that Cython 0.29.1 has now been released.

@kiwifb
Copy link
Member

kiwifb commented Nov 26, 2018

comment:32

Actually the Gentoo main tree caught up to me yesterday and 0.29.1 is in the main tree.

@jdemeyer

This comment has been minimized.

@jdemeyer jdemeyer changed the title Upgrade to Cython 0.29 Upgrade to Cython 0.29.1 Nov 27, 2018
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 27, 2018

Changed commit from 7ffa946 to 4c3e314

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 27, 2018

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

4c3e314Upgrade to Cython 0.29.1

@jdemeyer
Copy link
Author

comment:35

Not tested yet...

@embray
Copy link
Contributor

embray commented Nov 27, 2018

comment:36

Replying to @timokau:

For what its worth, here is the status of cython on various distros and these are the distros that package sage. Debian Unstable hasn't upgraded yet. Arch and Nix have. Gentoo isn't listed for some reason.

That's a cool site--I was not aware of it before. It doesn't look like it tracks conda-forge or cygwin packages either. I wonder how hard it would be to add trackers for new repositories to the site...

@timokau
Copy link
Contributor

timokau commented Nov 27, 2018

comment:37

Replying to @embray:

Replying to @timokau:

For what its worth, here is the status of cython on various distros and these are the distros that package sage. Debian Unstable hasn't upgraded yet. Arch and Nix have. Gentoo isn't listed for some reason.

That's a cool site--I was not aware of it before. It doesn't look like it tracks conda-forge or cygwin packages either. I wonder how hard it would be to add trackers for new repositories to the site...

Yeah its pretty neat. Nix uses a bot that automatically generates trivial package update PRs based on repology data. I think adding support for another repo would basically sonsist of (1) finding a relibale data source that gives current metadata about all packages and (2) converting that datasource into the format repology expects. There are already issues for conda-forge and cygwin.

@timokau
Copy link
Contributor

timokau commented Nov 27, 2018

comment:38

Replying to @jdemeyer:

With only this ticket applied (not the follow-up tickets), it requires only minor adjustments to support both Cython 0.28 and Cython 0.29.

So what you are saying is maybe more an argument to postpone the follow-up tickets until we know for sure that Debian is ready for Cython 0.29.

Back to topic: So in its current state this ticket is not backwards compatible?

@kiwifb
Copy link
Member

kiwifb commented Nov 27, 2018

comment:39

Replying to @timokau:

For what its worth, here is the status of cython on various distros and these are the distros that package sage. Debian Unstable hasn't upgraded yet. Arch and Nix have. Gentoo isn't listed for some reason.

The curating is imperfect and for some distros it is listed under "cython" and for other under "python:cython" https://repology.org/metapackage/python:cython/versions not much you can do about that.

@timokau
Copy link
Contributor

timokau commented Nov 27, 2018

comment:40

You can report it (I just did). Every time I've done that it has been resolved within a couple of hours.

@jdemeyer
Copy link
Author

comment:41

Replying to @timokau:

Back to topic: So in its current state this ticket is not backwards compatible?

No, it is not. The difference is really small though, packagers that insist on Cython 0.28 can easily revert this patch.

@timokau
Copy link
Contributor

timokau commented Nov 28, 2018

comment:42

Okay since the majority have already switched (all but debian I think) and its very possible that debian will have switched by the time 8.5 is released, I think we can go forward. Any other opinions?

@embray
Copy link
Contributor

embray commented Nov 29, 2018

comment:43

Replying to @jdemeyer:

Replying to @timokau:

Back to topic: So in its current state this ticket is not backwards compatible?

No, it is not. The difference is really small though, packagers that insist on Cython 0.28 can easily revert this patch.

I think that's a bit presumptuous. But I agree there's no reason for anyone to hold back up on updating Cython and that it should/will happen before long so I don't object to moving forward on this.

@timokau
Copy link
Contributor

timokau commented Nov 29, 2018

Reviewer: Timo Kaufmann

@vbraun
Copy link
Member

vbraun commented Nov 30, 2018

Changed branch from u/jdemeyer/upgrade_to_cython_0_29 to 4c3e314

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

9 participants