Compile Sage with Cython language_level=3str

[jdemeyer]

Cython 0.29 introduces a new language_level=3str, which is like language_level=3 except that strings are still str instead of unicode. It would be good to use this language_level in Sage to force Python 3 syntax in all Cython files.

CC: @embray @fchapoton

Component: cython

Author: Jeroen Demeyer

Branch/Commit: 58a30a8

Reviewer: Frédéric Chapoton

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

[embray]

comment:1

I am still confused by the whole language_level discussion. I tried following the discussion on the cython mailing list but I gave up.

If I understand correctly (and this is probably just restating what you wrote in the ticket description) language_level=3str means it's using Python 3 syntax in all Cython files, with the exception that unprefixed string literals are non-unicode strings on Python 2?

[embray]

comment:2

Regarding your deleted comment from the other ticket (maybe it was meant for this one) about non-ASCII strings in string literals (there was an example you used with μ). If a docstring contains non-ASCII characters then I definitely think we should be explicitly using u''.

[jdemeyer]

comment:3

Replying to @embray:

I am still confused by the whole language_level discussion. I tried following the discussion on the cython mailing list but I gave up.

If I understand correctly (and this is probably just restating what you wrote in the ticket description) language_level=3str means it's using Python 3 syntax in all Cython files, with the exception that unprefixed string literals are non-unicode strings on Python 2?

It's not only about string literals. With language_level=3, the global str means unicode:

sage: cython('''
....: # cython: language_level=3
....: print(str)
....: ''')
<type 'unicode'>

[jdemeyer]

comment:4

Replying to @embray:

If a docstring contains non-ASCII characters then I definitely think we should be explicitly using u''.

On the other hand, I do consider it a bug that Cython disallows "μ" with language_level=3str since that's valid in both Python 2 and Python 3 (with a different meaning though).

[embray]

comment:5

Replying to @jdemeyer:

Replying to @embray:

If a docstring contains non-ASCII characters then I definitely think we should be explicitly using u''.

On the other hand, I do consider it a bug that Cython disallows "μ" with language_level=3str since that's valid in both Python 2 and Python 3 (with a different meaning though).

Sure it's technically valid on Python 2, but not a great practice in most cases either, and with the different meaning there's a lot of potential for confusion around it so I think they're right to disallow it (even if it's unintentionally disallowed). But I also see your point.

[embray]

comment:6

Replying to @jdemeyer:

Replying to @embray:

I am still confused by the whole language_level discussion. I tried following the discussion on the cython mailing list but I gave up.

If I understand correctly (and this is probably just restating what you wrote in the ticket description) language_level=3str means it's using Python 3 syntax in all Cython files, with the exception that unprefixed string literals are non-unicode strings on Python 2?

It's not only about string literals. With language_level=3, the global str means unicode:

sage: cython('''
....: # cython: language_level=3
....: print(str)
....: ''')
<type 'unicode'>

Ok, sure. But "3str" means str is just always str right? I guess I like that approach if so; it meshes well with the approach we've been already taking in Sage.

[jdemeyer]

comment:7

Replying to @embray:

Ok, sure. But "3str" means str is just always str right?

Yes, str is str and "literals" are also of type str.

[jdemeyer]

Changed dependencies from #25292 to #25292, #26413

[jdemeyer]

Changed dependencies from #25292, #26413 to #25292, #26413, #26414

[jdemeyer]

Branch: u/jdemeyer/compile_sage_with_cython_language_level_3str

[sagetrac-git]

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

bcba4f1	Upgrade to cypari2-1.3.1
f77de1d	Upgrade to Cython 0.29
0ea70b5	Compile Sage with Cython language_level=3str

[sagetrac-git]

Commit: 0ea70b5

[sagetrac-git]

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

25c25ee	Various fixes to rational_power_parts()
5657422	Compile Sage with Cython language_level=3str
b3a8a88	Remove `__future__` statements from Cython files

[sagetrac-git]

Changed commit from 0ea70b5 to b3a8a88

[sagetrac-git]

Changed commit from b3a8a88 to 172bb57

[sagetrac-git]

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

732e8c5	Compile Sage with Cython language_level=3str
172bb57	Remove `__future__` statements from Cython files

[jdemeyer]

Changed dependencies from #25292, #26413, #26414 to none

[sagetrac-git]

Changed commit from 172bb57 to b821f77

[sagetrac-git]

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

7c36cdd	Explicitly set language_level=2 for a few modules
8fb3196	Compile Sage with Cython language_level=3str
b99e8da	Remove `__future__` statements from Cython files
b821f77	Doctest fixes

[embray]

comment:16

Ticket retargeted after milestone closed (if you don't believe this ticket is appropriate for the Sage 8.8 release please retarget manually)

[fchapoton]

comment:17

red branch

[jdemeyer]

comment:18

I know, but it's not ready for review anyway. This needs to wait for Cython 3.0

[fchapoton]

comment:26

Three patchbots seems to be happy, both with py2 and py3. Erik, would you approve a positive review ?

[fchapoton]

comment:27

Could please somebody else approve ? Or should I set to positive all by myself ?

[fchapoton]

Reviewer: Frédéric Chapoton

[fchapoton]

comment:28

In the absence of any reaction whatsoever, I am setting to positive.

[vbraun]

comment:29

merge conflict...

[embray]

comment:30

Replying to @fchapoton:

Could please somebody else approve ? Or should I set to positive all by myself ?

Yeah why shouldn't you? You reviewed the change and tested it, didn't you? You don't need me to do that.

[sagetrac-git]

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

0a34ee9

Merge branch 'public/ticket/26403' in 9.0.b1

[sagetrac-git]

Changed commit from 5a33013 to 0a34ee9

[fchapoton]

comment:32

conflicts resolved, setting back to positive

[vbraun]

comment:33

I'm getting the following failure on 32-bit:

File "src/sage/plot/plot3d/shapes.pyx", line 947, in sage.plot.plot3d.shapes.Sphere.get_grid
Failed example:
    Sphere(1).get_grid(100)
Expected:
    ([-10.0, ..., 0.0, ..., 10.0],
     [0.0, ..., 3.141592653589793, ..., 0.0])
Got:
    ([-10.0,
      -1.0471975511965976,
      -0.5235987755982988,
      6.1257422745431e-17,
      0.5235987755982989,
      1.0471975511965979,
      10.0],
     [0.0,
      1.0471975511965979,
      2.0943951023931957,
      3.141592653589793,
      4.188790204786391,
      5.235987755982989,
      0.0])
**********************************************************************
1 item had failures:
   1 of   3 in sage.plot.plot3d.shapes.Sphere.get_grid
    [162 tests, 1 failure, 43.51 s]
----------------------------------------------------------------------
sage -t --long src/sage/plot/plot3d/shapes.pyx  # 1 doctest failed
----------------------------------------------------------------------

[sagetrac-git]

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

58a30a8

trac 26403 fix 32bit doctest

[sagetrac-git]

Changed commit from 0a34ee9 to 58a30a8

[fchapoton]

comment:35

ok, fixed

[vbraun]

Changed branch from public/ticket/26403 to 58a30a8