Opened 10 years ago

Closed 10 years ago

#12343 closed defect (fixed)

Cleanup unexisting methods after #10263

Reported by: lftabera Owned by: tbd
Priority: major Milestone: sage-5.2
Component: factorization Keywords: regression, factorization
Cc: aapitzsch, mariah 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 jdemeyer)

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 lftabera 10 years ago.
12343_rebased.patch (5.6 KB) - added by jdemeyer 10 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 10 years ago by lftabera

  • Status changed from new to needs_review

comment:2 Changed 10 years ago by lftabera

  • Cc aapitzsch mariah added

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

comment:3 Changed 10 years ago by aapitzsch

  • Status changed from needs_review to needs_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: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 lftabera

comment:4 Changed 10 years ago by aapitzsch

  • Authors set to Luis Felipe Tabera Alonso
  • Reviewers set to André Apitzsch
  • Status changed from needs_work to positive_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 jdemeyer

  • Milestone changed from sage-5.3 to sage-5.2

comment:6 Changed 10 years ago by lftabera

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

Changed 10 years ago by jdemeyer

comment:7 Changed 10 years ago by jdemeyer

  • Description modified (diff)
  • Merged in set to sage-5.2.rc0
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.