Opened 5 years ago
Closed 5 years ago
#23648 closed defect (worksforme)
Py3: TypeError from Cython string conversion
Reported by:  rws  Owned by:  

Priority:  major  Milestone:  sageduplicate/invalid/wontfix 
Component:  python3  Keywords:  Py3, unicode 
Cc:  chapoton, embray  Merged in:  
Authors:  Frédéric Chapoton  Reviewers:  
Report Upstream:  N/A  Work issues:  
Branch:  public/23648 (Commits, GitHub, GitLab)  Commit:  7e014227489fa0d52a9528681833670ab7926904 
Dependencies:  #24222  Stopgaps: 
Description (last modified by )
With Py3, as reported in https://github.com/pynac/pynac/issues/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 SagePynac 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'
Change History (34)
comment:1 Changed 5 years ago by
 Component changed from cython to python3
 Keywords cython removed
comment:2 Changed 5 years ago by
comment:3 Changed 5 years ago by
It's just your typical unicode
vs. bytes
issue. Some API expects bytes
but is given unicode
instead.
comment:4 Changed 5 years ago by
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
comment:5 Changed 5 years ago by
Confirmed in the latest python3 build.
EDIT: And this is a major blocking point.
comment:6 Changed 5 years ago by
 Description modified (diff)
 Status changed from new to needs_info
comment:7 Changed 5 years ago by
Jeroen, can you please clarify where this should be fixed?
comment:8 Changed 5 years ago by
 Branch set to public/23648
 Commit set to 587938f63792af7a0d893e1126e5bc0cbd86c3a0
 Status changed from needs_info to needs_review
comment:9 Changed 5 years ago by
 Please remove
to_unicode = u
, that is not relevant here.
 I suggest to use
sys.getfilesystemencoding()
instead of hardcoding UTF8. The reason is that the conversions you are doing here are really analogous to the implicitbytes <> unicode
conversions that Python 3 does internally. In those cases, Python 3 mostly usessys.getfilesystemencoding()
.
comment:10 Changed 5 years ago by
 Maybe renaming the function to
string_to_bytes
to make it clear that it accepts only stringlike objects, it does not convert everything tobytes
.
 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 likesrc/sage/cpython/string.pyx
?
comment:11 Changed 5 years ago by
 Commit changed from 587938f63792af7a0d893e1126e5bc0cbd86c3a0 to 7e014227489fa0d52a9528681833670ab7926904
Branch pushed to git repo; I updated commit sha1. New commits:
7e01422  trac 23648

comment:12 Changed 5 years ago by
 Cc embray added
comment:13 Changed 5 years ago by
Alternatively, you could directly implement conversion char *
> str
(skipping the intermediate bytes
step) using PyUnicode_DecodeFSDefault
in Python 3.
comment:14 followups: ↓ 15 ↓ 16 Changed 5 years ago by
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 ?
comment:15 in reply to: ↑ 14 Changed 5 years ago by
comment:16 in reply to: ↑ 14 Changed 5 years ago by
comment:17 Changed 5 years ago by
Traceback please!
comment:18 Changed 5 years ago by
comment:19 Changed 5 years ago by
 Keywords unicode added
comment:20 Changed 5 years ago by
Here is some traceback (using ipython3 with a failed python3built):
In [3]: from sage.all import *  TypeError Traceback (most recent call last) <ipythoninput390a1b1fe16c5> in <module>() > 1 from sage.all import * /home/chapoton/sage3/local/lib/python3.6/sitepackages/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/sitepackages/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/sitepackages/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/sitepackages/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/sitepackages/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
comment:21 Changed 5 years ago by
ping ?
Once again, I am not able to make a cpdef function, so please do that.
comment:22 Changed 5 years ago by
I have created a new ticket that just wants to introduce the conversion tools: #24186.
comment:23 Changed 5 years ago by
No patchbot on this until now, strange.
comment:24 Changed 5 years ago by
Authors field is empty..
comment:25 Changed 5 years ago by
 Dependencies set to #24222
comment:26 Changed 5 years ago by
comment:27 Changed 5 years ago by
 Status changed from needs_review to needs_work
This should use the functions introduced in #24222.
comment:28 Changed 5 years ago by
 Milestone changed from sage8.2 to sageduplicate/invalid/wontfix
 Status changed from needs_work to needs_review
Let us close this one as invalid, please
comment:29 Changed 5 years ago by
Why? Is it already fixed?
comment:30 Changed 5 years ago by
It seems so. I am not able to reproduce that in ipython. And I am also currently no longer able to have a starting python3sage, sadly.
comment:31 Changed 5 years ago by
 Status changed from needs_review to needs_info
So we need more info, no?
comment:32 Changed 5 years ago by
 Status changed from needs_info to needs_review
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..
comment:33 Changed 5 years ago by
 Status changed from needs_review to positive_review
comment:34 Changed 5 years ago by
 Resolution set to worksforme
 Status changed from positive_review to closed
Indeed, this is fixed now.
so this is not a cython problem ? where is the issue then ?