Opened 11 years ago

Closed 10 years ago

#12343 closed defect (fixed)

Cleanup unexisting methods after #10263

Reported by: Luis Felipe Tabera Alonso Owned by: tbd
Priority: major Milestone: sage-5.2
Component: factorization Keywords: regression, factorization
Cc: aapitzsch, Mariah Lennox Merged in: sage-5.2.rc0
Authors: Luis Felipe Tabera Alonso Reviewers: André Apitzsch
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by Jeroen Demeyer)

In ticket #10623, some methods for integer factorization were moved to a separated file. Still, the sage library still calls some of them

_factor_trial_division in integer.pyx function squarefree_part

_factor_cunningham in finite_field_base.pyx and element_base.pyx

These methods should call to the proper functions in factorint.pyx

apply 12343_rebased.patch

Attachments (2)

12343.patch (5.5 KB) - added by Luis Felipe Tabera Alonso 10 years ago.
12343_rebased.patch (5.6 KB) - added by Jeroen Demeyer 10 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 10 years ago by Luis Felipe Tabera Alonso

Status: newneeds_review

comment:2 Changed 10 years ago by Luis Felipe Tabera Alonso

Cc: aapitzsch Mariah Lennox added

André, Mariah, since you were involved in #10623, whould you mind reviewing the ticket?

comment:3 Changed 10 years ago by aapitzsch

Status: needs_reviewneeds_work

Line 1002 in sage/rings/finite_rings/integer_mod.pyx is too long.

In the same file:

sage: a.nth_root(2,False,True,'Johnston',False)

should probably be

sage: a.nth_root(2,False,True,'Johnston',True)

to use cunningham.

``sage -i cunningham_tables`` without version number is enough.

Please, also remove trailing white spaces about which patchbot is complaining.

12343.patch
    12343.patch:14 +        sage: from sage.rings.factorint import factor_cunningham $
    12343.patch:65 +          factorization of ``n`` is computed. If cunningham is set to ``True``, $
Trailing whitespace inserted on 2 non-empty lines.

Changed 10 years ago by Luis Felipe Tabera Alonso

Attachment: 12343.patch added

comment:4 Changed 10 years ago by aapitzsch

Authors: Luis Felipe Tabera Alonso
Reviewers: André Apitzsch
Status: needs_workpositive_review

I suppose this is ready for review.

All tests passed including the -only_optional=cunningham ones.

Looks good to me!

comment:5 Changed 10 years ago by Jeroen Demeyer

Milestone: sage-5.3sage-5.2

comment:6 Changed 10 years ago by Luis Felipe Tabera Alonso

I wanted to check the compiled docs before setting it ready for review but you were faster :)

Changed 10 years ago by Jeroen Demeyer

Attachment: 12343_rebased.patch added

comment:7 Changed 10 years ago by Jeroen Demeyer

Description: modified (diff)
Merged in: sage-5.2.rc0
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.