py3: several string conversion fixes

[embray]

This fixes several (hardly exhaustive) string conversions around Sage such that functions and methods that returned str on Python 2 also return str on Python 3 (and the same for functions and methods that take str as arguments) using the new string utilities from #24222.

This also incorporates/replaces the fixes from #23812.

[embray]

With this, plus a few other small fixes (including fixes from other existing tickets merged in) I was able to get the Sage REPL up and 1 + 1 working.

[git]

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

​34ef6dc	Add HAVE_GMPY2 compile-time constant
​158d984	Force absolute import in have_module()
​7f06e71	Fix documentation
​3ac146c	Add a Cython compile-time constant for Python major version.
​f71fd41	Add new string conversion utilities.
​a5d5219	Add support for optional error handling.
​627d4c7	Add new string conversion utilities.
​25d6c32	Numerous string conversions from bytes to str and from str to bytes for Python 3

[embray]

Rebased on current version of #24222.

[jdemeyer]

Something is wrong: there are still the files src/sage/misc/string.*

[jdemeyer]

This is again a ticket with many small independent changes where a "partial positive review" like what I proposed on #24025 would make sense. I didn't like how we handled that, so how should we proceed here?

[embray]

Replying to jdemeyer:

Something is wrong: there are still the files src/sage/misc/string.*

Oops--part of the problem here is I'm moving between two different working trees, one for Python 2 and one for Python 3, so it can get confusing. I'll clean this up.

[embray]

Replying to jdemeyer:

This is again a ticket with many small independent changes where a "partial positive review" like what I proposed on #24025 would make sense. I didn't like how we handled that, so how should we proceed here?

I'd be fine if you proposed your changes and tossed it back to me as "needs review", rather than going immediately to "positive review".

[jdemeyer]

It seems strange to me to use locale.getpreferredencoding() for strings which do not depend on the locale. I would propose:

1. Use locale.getpreferredencoding() for strings which actually depend on the locale.

2. Use sys.getfilesystemencoding() for strings which actually depend on the file system encoding.

3. Use UTF-8 otherwise.

[git]

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

​64567aa	Make type checks explicit
​a358aea	Require a const char*--this is more consistent for use in cases where
​64a1c2d	Use a less unusual character in the tests
​1bf6709	Use r-strings for the docstrings since they contain slashes
​2bb8396	Clean up these declarations:
​6d73a41	Use more specific type declarations here.
​56f1c02	Numerous string conversions from bytes to str and from str to bytes for Python 3

[git]

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

​a285af4	Add new string conversion utilities.
​5b3719a	Add support for optional error handling.
​bf5f893	Make type checks explicit
​1bd27fc	Require a const char*--this is more consistent for use in cases where
​bd2d931	Use a less unusual character in the tests
​b32f000	Use r-strings for the docstrings since they contain slashes
​ccf7bcd	Clean up these declarations:
​dec9f3a	Use more specific type declarations here.
​a9ebae6	Numerous string conversions from bytes to str and from str to bytes for Python 3

[jdemeyer]

Is this needs_review again?

[embray]

Yeah, IIRC this was just a bunch of miscellaneous string conversions from some other tickets, but redone using the sage.cpython.string utilities.

[chapoton]

3 failing doctests in integer.pyx

[embray]

Interesting--when I added str_to_bytes I predicted we might later want to allow Python 2 unicode objects to be supported somehow, but put it off because I didn't think there was much code in Sage that already was expected to handle unicode (except in a few places like unicode art, etc.). But the tests that are failing here clearly show that there are some places it's expected to work.

So I see at least three options:

1. Modify str_to_bytes to treat unicode appropriate on Python 2
2. Add some new function like unicode_to_str that's analogous to str_to_bytes (I'm not crazy about this)
3. Just special case unicode on Python 2 where we do want to support it.

I lean towards 1. but I'm open to ideas.

[jdemeyer]

Replying to embray:

I lean towards 1.

I agree. We should just support bytes and unicode (in the Cython sense!) as input to str_to_bytes regardless of Python version. One complication is that the Python 3 code path of str_to_bytes uses some functions specific to Python 3, so we cannot just run that code on Python 2.

[embray]

Right, I'll give it a look though. It will just have to have a slightly different implementation for Python 2. Welcome back btw!

[jdemeyer]

I created a new ticket for this: #24475

[jdemeyer]

Replying to embray:

Right, I'll give it a look though. It will just have to have a slightly different implementation for Python 2.

I would rather avoid that. Instead of PyUnicode_EncodeLocale, you can just pass NULL as encoding to PyUnicode_AsEncodedString. That leaves PyUnicode_AsUTF8 which is really only used to translate the encoding and errors arguments. We could require that this is always of type str (so, not unicode on Python 2).

[jdemeyer]

Replying to embray:

I'll give it a look though.

I can also do that. Just let me know, to avoid double work.

[git]

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

​9cf556b	Rework str_to_bytes so that it accepts unicode on Python 2 and treats it same as str on Python 3.
​086e523	Numerous string conversions from bytes to str and from str to bytes for Python 3
​728a0bc	Use str_to_bytes for 'unicode' (which will be str on Python 3 and unicode on Python 2)
​c393e41	Minor Python 3 fix for sage.libs.flint.fmpz_poly

[chapoton]

is this in "needs_review" state ?

[git]

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

​0925f3b

A few additional string conversion fixes for sage.libs.ecl

[git]

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

​7e4d3cc

py3: fix pickling of integers and other minor bytes/str fixes

[git]

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

​998911f	Rework str_to_bytes so that it accepts unicode on Python 2 and treats it same as str on Python 3.
​c2c898e	Numerous string conversions from bytes to str and from str to bytes for Python 3
​0637cde	Use str_to_bytes for 'unicode' (which will be str on Python 3 and unicode on Python 2)
​c0158a9	Minor Python 3 fix for sage.libs.flint.fmpz_poly
​f415e2f	A few additional string conversion fixes for sage.libs.ecl
​1338ba4	py3: fix pickling of integers and other minor bytes/str fixes

[chapoton]

Looks good to me, globally, even if do not feel I understand everything in detail.

Jeroen, do you think we can move forward with this important ticket ?

[jdemeyer]

I always find it a bit unpleasant to review tickets with non-merged dependencies. So I would rather wait until the next beta.

Two immediate issues that I see:

1. In src/sage/libs/mpmath/utils.pyx you add an import that you don't use.

2. In several files, you import stuff from sage.cpython.string where you could use cimport.

[chapoton]

New commits:

​00f2776

Merge branch 'u/embray/python3/string-fixes' in 8.2.b4

[git]

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

​7d68d8c

some details of imports

[embray]

This does not work:

-from sage.cpython.string import FS_ENCODING, str_to_bytes
+from sage.cpython.string cimport FS_ENCODING, str_to_bytes

FS_ENCODING is can't be cimported.

[git]

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

​80b0d1f

fix import

[chapoton]

import corrected

[embray]

I rebased and squashed some of these changes.

---

New commits:

​cc214fd	Numerous string conversions from bytes to str and from str to bytes for Python 3
​f7e1153	Use str_to_bytes for 'unicode' (which will be str on Python 3 and unicode on Python 2)
​a4d6fe3	A few additional string conversion fixes for sage.libs.ecl
​d03a8d4	py3: fix pickling of integers and other minor bytes/str fixes
​e9a582e	import -> cimport where appropriate

[vbraun]

[sagelib-8.2.beta4] Error compiling Cython file:
[sagelib-8.2.beta4] ------------------------------------------------------------
[sagelib-8.2.beta4] ...
[sagelib-8.2.beta4] from sage.rings.finite_rings.finite_field_prime_modn import FiniteField_prime_modn
[sagelib-8.2.beta4] from sage.rings.finite_rings.finite_field_givaro import FiniteField_givaro
[sagelib-8.2.beta4] from sage.rings.finite_rings.finite_field_ntl_gf2e import FiniteField_ntl_gf2e
[sagelib-8.2.beta4] from sage.libs.pari.all import pari
[sagelib-8.2.beta4] from sage.libs.gmp.all cimport *
[sagelib-8.2.beta4] from sage.cpython.string cimport FS_ENCODING, str_to_bytes
[sagelib-8.2.beta4] ^
[sagelib-8.2.beta4] ------------------------------------------------------------
[sagelib-8.2.beta4] 
[sagelib-8.2.beta4] sage/libs/singular/singular.pyx:40:0: 'sage/cpython/string/FS_ENCODING.pxd' not found
[sagelib-8.2.beta4] 
[sagelib-8.2.beta4] Error compiling Cython file:
[sagelib-8.2.beta4] ------------------------------------------------------------
[sagelib-8.2.beta4] ...
[sagelib-8.2.beta4]     lib = SINGULAR_SO
[sagelib-8.2.beta4] 
[sagelib-8.2.beta4]     if not os.path.exists(lib):
[sagelib-8.2.beta4]         raise ImportError("cannot locate Singular library ({})".format(lib))
[sagelib-8.2.beta4] 
[sagelib-8.2.beta4]     lib = str_to_bytes(lib, FS_ENCODING, "surrogateescape")
[sagelib-8.2.beta4]                            ^
[sagelib-8.2.beta4] ------------------------------------------------------------
[sagelib-8.2.beta4] 
[sagelib-8.2.beta4] sage/libs/singular/singular.pyx:778:28: 'FS_ENCODING' is not a constant, variable or function identifier
[sagelib-8.2.beta4] Traceback (most recent call last):
[sagelib-8.2.beta4]   File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/Cython/Build/Dependencies.py", line 1179, in cythonize_one_helper
[sagelib-8.2.beta4]     return cythonize_one(*m)
[sagelib-8.2.beta4]   File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/Cython/Build/Dependencies.py", line 1161, in cythonize_one
[sagelib-8.2.beta4]     raise CompileError(None, pyx_file)
[sagelib-8.2.beta4] CompileError: sage/libs/singular/singular.pyx

[chapoton]

Volker, did you use the latest branch ?

[embray]

He didn't. Setting this back to positive review since this otherwise fixes Jeroen's issue.