Opened 6 years ago
Closed 5 years ago
#22208 closed enhancement (fixed)
Conversion from SR to number fields
Reported by:  kcrisman  Owned by:  

Priority:  minor  Milestone:  sage8.0 
Component:  number fields  Keywords:  
Cc:  slelievre  Merged in:  
Authors:  Jeroen Demeyer  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  beed700 (Commits, GitHub, GitLab)  Commit:  beed7001cf8f8b2a18b77b591e05334c2ec9a91e 
Dependencies:  Stopgaps: 
Description (last modified by )
I'm not sure if this is kosher, but I would like
sage: G = GaussianIntegers() sage: G(2+I)
to "just work".
Change History (23)
comment:1 Changed 6 years ago by
comment:2 followup: ↓ 3 Changed 6 years ago by
This could help: #18036
comment:3 in reply to: ↑ 2 Changed 6 years ago by
This could help: #18036
Haha, I was sure that one had been merged by others long ago! I'll at least put a xref to this one there.
comment:4 Changed 5 years ago by
 Milestone changed from sage7.6 to sageduplicate/invalid/wontfix
 Resolution set to duplicate
 Status changed from new to closed
comment:5 Changed 5 years ago by
 Cc slelievre added
Note that the following also works (in Sage 7.5.1).
sage: G = GaussianIntegers() sage: G('2 + I') I + 2
comment:6 Changed 5 years ago by
 Description modified (diff)
 Resolution duplicate deleted
 Status changed from closed to new
 Summary changed from Allow immediate coercion from SR to Gaussian? to Conversion from SR to number fields
comment:7 Changed 5 years ago by
I'm reopening this because there are two ways to interpret this ticket:
 Allow
G(2 + I)
to just work, using the symbolI
.
 Allow
G(expr)
to work, whereexpr
is some symbolic expression representing a number field element.
Only the first one would be fixed by #18036.
comment:8 Changed 5 years ago by
 Milestone changed from sageduplicate/invalid/wontfix to sage8.0
comment:9 Changed 5 years ago by
 Branch set to u/jdemeyer/conversion_from_sr_to_number_fields
comment:10 Changed 5 years ago by
 Commit set to 0128efca77e3faf7cd18749ad49f610958008021
 Status changed from new to needs_review
New commits:
0128efc  Conversion from AA, QQbar and SR to number fields

comment:11 Changed 5 years ago by
Can you be more specific than except Exception
?
comment:12 Changed 5 years ago by
I'll try to just drop the except Exception
completely.
comment:13 Changed 5 years ago by
 Commit changed from 0128efca77e3faf7cd18749ad49f610958008021 to 47845a424847db5ef208298d4e7fdbd84c695b03
Branch pushed to git repo; I updated commit sha1. New commits:
47845a4  Further fixes to conversion of nonnumberfield elements

comment:14 Changed 5 years ago by
In the end, I did some more refactoring, including removing some duplicate code.
comment:15 followup: ↓ 16 Changed 5 years ago by
Sorry, to much algebra, cannot review.
comment:16 in reply to: ↑ 15 Changed 5 years ago by
Sorry, to much algebra, cannot review.
Most of the algebra is just cutting and pasting some examples, though, so those wouldn't need explicit review ...
comment:17 Changed 5 years ago by
 Reviewers set to Travis Scrimshaw
I doublechecked, and the algebra LGTM. I do have one minor docstring fix:
"""  Convert a nonnumber field element x into this number field. + Convert a nonnumber field element ``x`` into this number field. INPUT:   ``x``  a non number field element x, e.g., a list, integer,  rational, or polynomial. +  ``x``  a non number field element, e.g., a list, integer, + rational, or polynomial
Once that is done, you can set a positive review on my behalf.
comment:18 Changed 5 years ago by
 Commit changed from 47845a424847db5ef208298d4e7fdbd84c695b03 to 4a3e6386daf2ea7c691810cab70373339f5c161d
Branch pushed to git repo; I updated commit sha1. New commits:
4a3e638  Minor doc fix

comment:19 Changed 5 years ago by
 Status changed from needs_review to positive_review
comment:21 Changed 5 years ago by
 Commit changed from 4a3e6386daf2ea7c691810cab70373339f5c161d to beed7001cf8f8b2a18b77b591e05334c2ec9a91e
Branch pushed to git repo; I updated commit sha1. New commits:
beed700  Merge tag '8.1.beta0' into t/22208/conversion_from_sr_to_number_fields

comment:22 Changed 5 years ago by
 Status changed from needs_work to positive_review
comment:23 Changed 5 years ago by
 Branch changed from u/jdemeyer/conversion_from_sr_to_number_fields to beed7001cf8f8b2a18b77b591e05334c2ec9a91e
 Resolution set to fixed
 Status changed from positive_review to closed
Looks like a conversion as in
symbolic/expression_conversions.py
should do the trick.