Ticket #6091 (closed defect: fixed)
[with new patch, positive review] syntax extended for subfields of number fields
| Reported by: | fwclarke | Owned by: | was |
|---|---|---|---|
| Priority: | major | Milestone: | sage-4.0.1 |
| Component: | number theory | Keywords: | |
| Cc: | Work issues: | ||
| Report Upstream: | Reviewers: | Mike Hansen | |
| Authors: | Francis Clarke | Merged in: | 4.0.1.rc1 |
| Dependencies: | Stopgaps: |
Description
At present
sage: C.<z> = CyclotomicField(20) sage: D.<w>, phi = C.subfield(z^4)
fails.
This is simply because the code uses the name name instead of the name names. The patch fixes this, and does the same for change_generator (with doctests).
Attachments
-
trac_6091.patch
(3.8 KB) -
added by fwclarke 4 years ago.
-
trac_6091_revised.patch
(3.5 KB) -
added by fwclarke 4 years ago.
- replaces previous
Change History
comment:1 Changed 4 years ago by davidloeffler
- Owner changed from tbd to was
- Summary changed from [with patch, needs review] syntax extended for subfields of number fields to [with patch, positive review] syntax extended for subfields of number fields
- Component changed from algebra to number theory
- Milestone set to sage-4.0.1
Positive review. Patch applies fine to 4.0.rc1, all tests in sage/rings/number_field and doc/en/bordeaux_2008 pass; and the new syntax is a very useful thing to have.
David
comment:2 follow-up: ↓ 3 Changed 4 years ago by mhansen
- Summary changed from [with patch, positive review] syntax extended for subfields of number fields to [with patch, needs work] syntax extended for subfields of number fields
This will break all old code that uses the name= syntax.
There is a decorator at sage.plot.misc.rename_keyword that could be use to rename a 'name' keyword argument to 'names'. A useful thing to do would be to add a flag to that decorator so that a deprecation warning would be thrown whenever a rename is done.
comment:3 in reply to: ↑ 2 Changed 4 years ago by fwclarke
- Summary changed from [with patch, needs work] syntax extended for subfields of number fields to [with new patch, needs review] syntax extended for subfields of number fields
Replying to mhansen:
This will break all old code that uses the name= syntax.
Point taken.
There is a decorator at sage.plot.misc.rename_keyword that could be use to rename a 'name' keyword argument to 'names'. A useful thing to do would be to add a flag to that decorator so that a deprecation warning would be thrown whenever a rename is done.
I think in this case there's a simpler solution, already used in the NumberFieldfunction, which allows either name or names; see the replacement patch.
comment:4 Changed 4 years ago by mhansen
- Status changed from new to closed
- Resolution set to fixed
- Summary changed from [with new patch, needs review] syntax extended for subfields of number fields to [with new patch, positive review] syntax extended for subfields of number fields
While it is a solution, I think it's a bit more hackish. We should really clean these things up a bit. But, I'm okay with this going in.
Merged in 4.0.1.rc1.
