Opened 7 years ago

Closed 7 years ago

#13766 closed defect (duplicate)

No conversion from unit group to number field.

Reported by: mderickx Owned by: davidloeffler
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: number fields Keywords:
Cc: mstreng Merged in:
Authors: Reviewers: Marco Streng, Maarten Derickx
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by mstreng)

* already fixed by #13687, apply no patches *

Currently we have:

sage: L.<a>=CyclotomicField(7)
sage: a in L.unit_group()
False

This is because:

sage: a1=L.unit_group()(a)
sage: L(a1)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)

/Users/maarten/Dropbox/Artikelen/MazurKamiennyDerickx/tex/<ipython console> in <module>()

/Applications/sage/local/lib/python2.6/site-packages/sage/structure/parent.so in sage.structure.parent.Parent.__call__ (sage/structure/parent.c:6820)()

/Applications/sage/local/lib/python2.6/site-packages/sage/structure/coerce_maps.so in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (sage/structure/coerce_maps.c:3254)()

/Applications/sage/local/lib/python2.6/site-packages/sage/structure/coerce_maps.so in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (sage/structure/coerce_maps.c:3157)()

/Applications/sage/local/lib/python2.6/site-packages/sage/rings/number_field/number_field.pyc in _element_constructor_(self, x)
   6974             return self._coerce_from_str(x)
   6975         else:
-> 6976             return self._coerce_non_number_field_element_in(x)
   6977 
   6978     # TODO:

/Applications/sage/local/lib/python2.6/site-packages/sage/rings/number_field/number_field.pyc in _coerce_non_number_field_element_in(self, x)
   5064         except (TypeError, AttributeError), msg:
   5065             pass
-> 5066         raise TypeError, type(x)
   5067 
   5068     def _coerce_from_str(self, x):

TypeError: <class 'sage.groups.abelian_gps.abelian_group_element.AbelianGroupElement'>

* already fixed by #13687, apply no patches *

Attachments (1)

trac13766_unit_group_conversion.patch (1.7 KB) - added by mderickx 7 years ago.

Download all attachments as: .zip

Change History (13)

comment:1 Changed 7 years ago by mderickx

  • Component changed from PLEASE CHANGE to number fields
  • Owner changed from tbd to davidloeffler
  • Type changed from PLEASE CHANGE to defect

comment:2 Changed 7 years ago by mderickx

  • Cc mstreng added

comment:3 Changed 7 years ago by mderickx

  • Summary changed from no conversion from unit group to number field to No conversion from unit group to number field.

comment:4 Changed 7 years ago by robharron

This works for me in sage 5.8. This ticket should be closed.

comment:5 Changed 7 years ago by mderickx

  • Status changed from new to needs_review

Indeed, I can confirm that it works now. I added a patch that contains a doctest in order to make sure this doesn't break in the future.

Changed 7 years ago by mderickx

comment:6 Changed 7 years ago by mstreng

is there no such doctest in the patch that fixed this?

comment:7 follow-up: Changed 7 years ago by mderickx

Well, I have no idea wich ticket fixed this. And I couldn't find a doctest which tests this somewhere.

comment:8 in reply to: ↑ 7 Changed 7 years ago by mstreng

Replying to mderickx:

Well, I have no idea wich ticket fixed this.

#13687

And I couldn't find a doctest which tests this somewhere.

Coercion from the abstract unit group to the number field is tested by #13687.

sage: UK.gen(0) + K.one()   # coerce abstract generator into number field

Conversion the other way around already had a test.

sage: [UK(u) for u in (x^4-1).roots(K,multiplicities=False)]

The coercion framework will then make sure that "in" also works correctly. I don't think another test is necessary.

Enhancing the documentation by explaining that "in" works in this case is nice, but then make sure the English grammar is correct in the patch as well. And there are issues like the fact that u0^6 may depend on the pari version, since any even exponent is correct, so this test will break later.

I propose to just close this ticket as duplicate instead :)

comment:9 Changed 7 years ago by mderickx

  • Milestone changed from sage-5.9 to sage-duplicate/invalid/wontfix
  • Status changed from needs_review to positive_review

comment:10 Changed 7 years ago by mderickx

Ok, I changed it to duplicate/invalid/wontfix and gave it positive review status so that Jeroen can close this.

comment:11 follow-up: Changed 7 years ago by mstreng

  • Description modified (diff)

(just making it a bit clearer/easier for Jeroen)

comment:12 in reply to: ↑ 11 Changed 7 years ago by jdemeyer

  • Resolution set to duplicate
  • Reviewers set to Marco Streng, Maarten Derickx
  • Status changed from positive_review to closed

Replying to mstreng:

(just making it a bit clearer/easier for Jeroen)

Absolutely, thanks!

Note: See TracTickets for help on using tickets.