Opened 21 months ago
Closed 20 months ago
#28321 closed defect (fixed)
py3: rationals can not be initialized from a pair of big Python ints
Reported by:  vdelecroix  Owned by:  

Priority:  critical  Milestone:  sage8.9 
Component:  python3  Keywords:  
Cc:  Merged in:  
Authors:  Vincent Delecroix, Jeroen Demeyer  Reviewers:  Jeroen Demeyer, Vincent Delecroix 
Report Upstream:  N/A  Work issues:  
Branch:  ef9ae69 (Commits, GitHub, GitLab)  Commit:  ef9ae6904a4620e36c75866f94cc337d5e524ef2 
Dependencies:  Stopgaps: 
Description
sage: QQ((2r**100,3r**100))  OverflowError Traceback (most recent call last) <ipythoninput2091103d07f87> in <module>() > 1 QQ((2**Integer(100),3**Integer(100))) /opt/sage/sagepy3gcc/local/lib/python3.7/sitepackages/sage/structure/parent.pyx in sage.structure.parent.Parent.__call__ (build/cythonized/sage/structure/parent.c:9197)() 898 if mor is not None: 899 if no_extra_args: > 900 return mor._call_(x) 901 else: 902 return mor._call_with_args(x, args, kwds) /opt/sage/sagepy3gcc/local/lib/python3.7/sitepackages/sage/structure/coerce_maps.pyx in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (build/cythonized/sage/structure/coerce_maps.c:4556)() 160 print(type(C), C) 161 print(type(C._element_constructor), C._element_constructor) > 162 raise 163 164 cpdef Element _call_with_args(self, x, args=(), kwds={}): /opt/sage/sagepy3gcc/local/lib/python3.7/sitepackages/sage/structure/coerce_maps.pyx in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (build/cythonized/sage/structure/coerce_maps.c:4448)() 155 cdef Parent C = self._codomain 156 try: > 157 return C._element_constructor(x) 158 except Exception: 159 if print_warnings: /opt/sage/sagepy3gcc/local/lib/python3.7/sitepackages/sage/rings/rational.pyx in sage.rings.rational.Rational.__init__ (build/cythonized/sage/rings/rational.c:6386)() 520 """ 521 if x is not None: > 522 self.__set_value(x, base) 523 524 def __reduce__(self): /opt/sage/sagepy3gcc/local/lib/python3.7/sitepackages/sage/rings/rational.pyx in sage.rings.rational.Rational.__set_value (build/cythonized/sage/rings/rational.c:7608)() 615 if isinstance(num, int) and isinstance(denom, int): 616 if denom >= 0: > 617 mpq_set_si(self.value, num, denom) 618 else: 619 mpq_set_si(self.value, num, denom) OverflowError: Python int too large to convert to C long
Change History (12)
comment:1 Changed 21 months ago by
 Branch set to u/vdelecroix/28321
 Commit set to 50cce861e438f965a283dfbf6b83d3aa06732559
 Status changed from new to needs_review
comment:2 Changed 21 months ago by
 Commit changed from 50cce861e438f965a283dfbf6b83d3aa06732559 to ab164da54b408b68f67e194b1403010d56e1fce1
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
ab164da  fix rational constructor

comment:3 Changed 21 months ago by
 Component changed from basic arithmetic to python3
 Summary changed from rationals can not be initialized from a pair of big Python ints to py3: rationals can not be initialized from a pair of big Python ints
Clarifying that this is a Python 3 issue.
comment:4 Changed 21 months ago by
Positive review if this passes tests also on Python 2.
comment:5 Changed 21 months ago by
 Reviewers set to Jeroen Demeyer
comment:6 Changed 20 months ago by
mpz_set_pylong
with Python2 seems unhappy when providing int
... do you have an idea on how to sort this out?
I just ran into this problem again with #28370.
comment:7 Changed 20 months ago by
comment:8 Changed 20 months ago by
 Branch changed from u/vdelecroix/28321 to u/jdemeyer/28321
comment:9 Changed 20 months ago by
 Commit changed from ab164da54b408b68f67e194b1403010d56e1fce1 to ef9ae6904a4620e36c75866f94cc337d5e524ef2
comment:10 Changed 20 months ago by
I see. Thanks!
comment:11 Changed 20 months ago by
 Reviewers changed from Jeroen Demeyer to Jeroen Demeyer, Vincent Delecroix
 Status changed from needs_review to positive_review
Tests pass with your version!
comment:12 Changed 20 months ago by
 Branch changed from u/jdemeyer/28321 to ef9ae6904a4620e36c75866f94cc337d5e524ef2
 Resolution set to fixed
 Status changed from positive_review to closed
Note: See
TracTickets for help on using
tickets.
New commits:
fix rational constructor