Opened 12 years ago

Closed 11 years ago

#9459 closed enhancement (fixed)

Implement a generic radical() function

Reported by: jdemeyer Owned by: was
Priority: major Milestone: sage-4.6.2
Component: number theory Keywords:
Cc: mhansen Merged in: sage-4.6.2.alpha0
Authors: Jeroen Demeyer Reviewers: John Cremona
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

Right now, there is a function radical() as member of IntegerRing_class. But there is no generic radical() function:

sage: radical(100)
---------------------------------------------------------------------------
NameError                                 Traceback (most recent call last)

/usr/local/src/sage-4.4.4/devel/sage-test/<ipython console> in <module>()

NameError: name 'radical' is not defined

Attachments (1)

9459.patch (8.7 KB) - added by jdemeyer 12 years ago.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 12 years ago by jdemeyer

  • Status changed from new to needs_review

The attached patch adds a radical() function, functions Factorization.radical() and Factorization.radical_value(). It also fixes a bug in the radical of a polynomial where the content was not accounted for.

It also changes the output of radical(), in a way which I think makes more sense: radical(0) now gives an error instead of returning 1 and the radical of a negative number is positive (instead of negative).

In order for all doctests to succeed, you need to apply also the patches for #9450.

comment:2 Changed 12 years ago by mhansen

  • Cc mhansen added

comment:3 Changed 12 years ago by kcrisman

For what it's worth, groups and algebras have radicals too, so it might be worth putting a #TODO in the main definition that one might want radical(G,type='nilpotent') to work here if that ever is implemented/wrapped nicely (e.g. see here), so that future developers do not break the possibility of that syntax.

comment:4 Changed 12 years ago by cremona

  • Reviewers set to John Cremona
  • Status changed from needs_review to positive_review

Applies well to 4.6.1.alpha1 and all tests pass.

comment:5 follow-up: Changed 12 years ago by jdemeyer

  • Milestone changed from sage-4.6.1 to sage-4.6.2

Thanks, this is something I did in Leiden and totally forgot about.

comment:6 in reply to: ↑ 5 Changed 12 years ago by cremona

Replying to jdemeyer:

Thanks, this is something I did in Leiden and totally forgot about.

I cannot see a reason not to include it, despite the comment from kcrisman.

Changed 12 years ago by jdemeyer

comment:7 Changed 11 years ago by jdemeyer

  • Merged in set to sage-4.6.2.alpha0
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.