Opened 6 years ago
Closed 6 years ago
#17190 closed defect (fixed)
Error in conversion from RR['x,y'] to RR['x']
Reported by:  defeo  Owned by:  

Priority:  major  Milestone:  sage6.9 
Component:  commutative algebra  Keywords:  polynomials RR 
Cc:  Merged in:  
Authors:  Bruno Grenet  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  6315d74 (Commits, GitHub, GitLab)  Commit:  6315d74ebf51bc82c26d6dafd389f52d895c8d78 
Dependencies:  Stopgaps: 
Description (last modified by )
Conversion of univariate polynomials from the bivariate ring RR['x,y']
fails:
sage: RR['x'](RR['x,y'].0)  ValueError Traceback (most recent call last) ... ValueError: max() arg is an empty sequence
This does not seem to affect other rings:
sage: QQ['x'](QQ['x,y'].0) x sage: CC['x'](CC['x,y'].0) x sage: RDF['x'](RDF['x,y'].0) x
Change History (9)
comment:1 Changed 6 years ago by
 Description modified (diff)
comment:2 Changed 6 years ago by
 Description modified (diff)
comment:3 Changed 6 years ago by
 Branch set to u/bruno/t17190_error_conversion
comment:4 Changed 6 years ago by
 Commit set to c546e17684c29b2e5f108440205833b55e7622d5
 Status changed from new to needs_review
comment:5 Changed 6 years ago by
 Commit changed from c546e17684c29b2e5f108440205833b55e7622d5 to fe6f95c764bbc69663ac6a68cc92723426ec40d2
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
fe6f95c  #17190: Authorize empty dictionary in initialization of polynomial over RR

comment:6 followup: ↓ 8 Changed 6 years ago by
 Milestone changed from sage6.4 to sage6.9
 Reviewers set to Travis Scrimshaw
LGTM modulo adding a blank line here:
Check that :trac:`17190` is fixed::
+
sage: RR['x']({})
0
Once you make this change, you can set a positive review on my behalf. (Also, could you edit you post above to show the result is 0?)
comment:7 Changed 6 years ago by
 Commit changed from fe6f95c764bbc69663ac6a68cc92723426ec40d2 to 6315d74ebf51bc82c26d6dafd389f52d895c8d78
Branch pushed to git repo; I updated commit sha1. New commits:
6315d74  #17190: Add missing blank line

comment:8 in reply to: ↑ 6 Changed 6 years ago by
 Status changed from needs_review to positive_review
Replying to tscrim:
LGTM modulo adding a blank line here:
Check that :trac:`17190` is fixed:: + sage: RR['x']({}) 0
Once you make this change, you can set a positive review on my behalf. (Also, could you edit you post above to show the result is 0?)
Thanks for the quick review! (I edited my post to make it correct...)
comment:9 Changed 6 years ago by
 Branch changed from u/bruno/t17190_error_conversion to 6315d74ebf51bc82c26d6dafd389f52d895c8d78
 Resolution set to fixed
 Status changed from positive_review to closed
[EDIT] As one can notice, I messed up the history with some completely unrelated commits. I am trying to undo this... The relevant code is the latest commit only! [/EDIT]Should be OK now!The problem was not in conversion, but rather in initialization of univariate polynomials over
RR
, with an empty dictionary. Before:After:
As a result, we also have:
Details
In the initialization of a polynomial over
RR
, if the "data" is given as a dictionary, this dictionary is converted to a list. Before the patch, there was an attempt to computemax(x.keys())
wherex
is the dictionary. Ifx
is empty, this yields an error. To correct the error, I could have add a test. Yet, a function is already specifically defined in the classPolynomial
to perform this conversiondict > list
. The new code just uses this function.Note that I corrected some slight formatting issues in the docstring in other (unrelated) parts of the same file
src/sage/rings/polynomials/polynomial_real_mpfr_dense.pyx
.New commits: (Note: Irrelevant commits. The only correct commit is in comment:5 below)
Rebased on 6.9beta1
One last zero_element removed + correct one doctest
Remove useless colons
#19171: New method divides
#19172: Rewrite valuation for dense polynomials
#19172: valuation for sparse polynomials
#17190: Authorize empty dictionary in initialization of polynomial over RR