Py3: TypeError from Cython string conversion

[rwst]

With Py3, as reported in pynac/pynac#271

In [2]: import sage
In [3]: from sage.libs.pynac.constant import PynacConstant
In [4]: f = PynacConstant('foo', 'foo', 'real')
...
TypeError: expected bytes, str found

It's is in this part of the Sage-Pynac interface:

  /* "sage/libs/pynac/constant.pyx":68
 *             self.pointer = <GConstant *>&g_NaN
 *         else:
>>> *          self._object = new GConstant(name, ConstantEvalf, texname, domain)             # <<<<<<<<<<<<<<
 *             self.pointer = self._object
 * 
 */
  /*else*/ {
    __pyx_t_2 = __Pyx_PyObject_AsWritableString(__pyx_v_name); if (unlikely((!__pyx_t_2) && PyErr_Occurred())) __PYX_ERR(0, 68, __pyx_L1_error)
    __pyx_t_3 = __Pyx_PyObject_AsWritableString(__pyx_v_texname); if (unlikely((!__pyx_t_3) && PyErr_Occurred())) __PYX_ERR(0, 68, __pyx_L1_error)
    __pyx_v_self->_object = new constant(__pyx_t_2, ConstantEvalf, __pyx_t_3, __pyx_v_domain);

Apparently the error comes from this function (or below): https://github.com/cython/cython/blob/master/Cython/Utility/TypeConversion.c#L202

EDIT: also, one currently gets

In [28]: f = PynacConstant(b'foo', b'foo', 'real')

In [29]: f.__repr__()
Out[29]: b'foo'

Depends on #24222

CC: @fchapoton @embray

Component: python3

Keywords: unicode

Author: Frédéric Chapoton

Branch/Commit: public/23648 @ 7e01422

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

[fchapoton]

comment:2

so this is not a cython problem ? where is the issue then ?

[jdemeyer]

comment:3

It's just your typical unicode vs. bytes issue. Some API expects bytes but is given unicode instead.

[fchapoton]

comment:4

well, same kind of thing as here then ?

cypari2/auto_gen.pxi in cypari2.gen.Gen_auto.Polrev()

cypari2/pari_instance.pyx in cypari2.pari_instance.get_var()

TypeError: string argument without an encoding

Hopefully this one will be fixed by #23518

[fchapoton]

comment:5

Confirmed in the latest python3 build.

EDIT: And this is a major blocking point.

[rwst]

comment:7

Jeroen, can you please clarify where this should be fixed?

[fchapoton]

Commit: 587938f

[fchapoton]

New commits:

069fd36	new method "to_bytes" and change in pynac constant
587938f	adding an alias for u

[fchapoton]

Branch: public/23648

[jdemeyer]

comment:9

1. Please remove to_unicode = u, that is not relevant here.

2. I suggest to use sys.getfilesystemencoding() instead of hard-coding UTF-8. The reason is that the conversions you are doing here are really analogous to the implicit bytes <-> unicode conversions that Python 3 does internally. In those cases, Python 3 mostly uses sys.getfilesystemencoding().

[jdemeyer]

comment:10

3. Maybe renaming the function to string_to_bytes to make it clear that it accepts only string-like objects, it does not convert everything to bytes.

4. For efficiency, I would implement this in a Cython file as cpdef functions such that Cython code can benefit from fast calls. Maybe a new file like src/sage/cpython/string.pyx?

[sagetrac-git]

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

7e01422

trac 23648

[sagetrac-git]

Changed commit from 587938f to 7e01422

[jdemeyer]

comment:13

Alternatively, you could directly implement conversion char * -> str (skipping the intermediate bytes step) using PyUnicode_DecodeFSDefault in Python 3.

[fchapoton]

comment:14

How is your last comment related to #23857 ?

I am not able to turn the branche here easily into a cpdef function. Could one of you do that please ?

[jdemeyer]

comment:15

Replying to @fchapoton:

How is your last comment related to #23857 ?

It is not strictly related to #23857. It is just that #23857 also fixes some bytes vs str bugs, but only for C++ code.

[fchapoton]

comment:16

Replying to @fchapoton:

How is your last comment related to #23857 ?

I am not able to turn the branche here easily into a cpdef function. Could one of you do that please ?

ping ?

[jdemeyer]

comment:17

Traceback please!

[jdemeyer]

comment:18

See also https://groups.google.com/d/msg/sage-devel/H0t9l6XdfPI/5ss5I5A4CAAJ

[fchapoton]

Changed keywords from none to unicode

[fchapoton]

comment:20

Here is some traceback (using ipython3 with a failed python3-built):

In [3]: from sage.all import *
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-3-90a1b1fe16c5> in <module>()
----> 1 from sage.all import *

/home/chapoton/sage3/local/lib/python3.6/site-packages/sage/all.py in <module>()
    100 from sage.matrix.all     import *
    101 
--> 102 from sage.symbolic.all   import *
    103 from sage.modules.all    import *
    104 from sage.monoids.all    import *

/home/chapoton/sage3/local/lib/python3.6/site-packages/sage/symbolic/all.py in <module>()
      5 
      6 from .ring import SR
----> 7 from .constants import (pi, e, NaN, golden_ratio, log2, euler_gamma, catalan,
      8                        khinchin, twinprime, mertens, glaisher)
      9 from .expression import Expression, solve_diophantine

/home/chapoton/sage3/local/lib/python3.6/site-packages/sage/symbolic/constants.py in <module>()
    842         return sympy.GoldenRatio
    843 
--> 844 golden_ratio = GoldenRatio().expression()
    845 
    846 class Log2(Constant):

/home/chapoton/sage3/local/lib/python3.6/site-packages/sage/symbolic/constants.py in __init__(self, name)
    772                            kash='(1+Sqrt(5))/2', giac='(1+sqrt(5))/2')
    773         Constant.__init__(self, name, conversions=conversions,
--> 774                           latex=r'\phi', domain='positive')
    775 
    776     def minpoly(self, bits=None, degree=None, epsilon=0):

/home/chapoton/sage3/local/lib/python3.6/site-packages/sage/symbolic/constants.py in __init__(self, name, conversions, latex, mathml, domain)
    286 
    287         from sage.libs.pynac.constant import PynacConstant
--> 288         self._pynac = PynacConstant(self._name, self._latex, self._domain)
    289         self._serial = self._pynac.serial()
    290         constants_table[self._serial] = self

/home/chapoton/sage3/src/sage/libs/pynac/constant.pyx in sage.libs.pynac.constant.PynacConstant.__cinit__ (build/cythonized/sage/libs/pynac/constant.cpp:2527)()

TypeError: expected bytes, str found

[fchapoton]

comment:21

ping ?

Once again, I am not able to make a cpdef function, so please do that.

[fchapoton]

comment:22

I have created a new ticket that just wants to introduce the conversion tools: #24186.

[rwst]

comment:23

No patchbot on this until now, strange.

[fchapoton]

comment:24

Authors field is empty..

[rwst]

Dependencies: #24222

[jdemeyer]

Author: Frédéric Chapoton

[jdemeyer]

comment:27

This should use the functions introduced in #24222.

[fchapoton]

comment:28

Let us close this one as invalid, please

[jdemeyer]

comment:29

Why? Is it already fixed?

[fchapoton]

comment:30

It seems so. I am not able to reproduce that in ipython. And I am also currently no longer able to have a starting python3-sage, sadly.

[rwst]

comment:31

So we need more info, no?

[fchapoton]

comment:32

No, no need for more info. Maybe I was not clear enough.

Using an ipython shell, I can do exactly what I did in the first part of the ticket description. And there is no longer any failure. So the problem has been fixed somewhere in one of the previous py3 tickets.

Believe me, one should close this one and concentrate on the many other py3 tickets..

[embray]

comment:34

Indeed, this is fixed now.