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: |
Description (last modified by )
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)
Change History (9)
comment:1 Changed 10 years ago by
Status: | new → needs_review |
---|
comment:2 Changed 10 years ago by
Cc: | aapitzsch Mariah Lennox added |
---|
comment:3 Changed 10 years ago by
Status: | needs_review → 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 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
Attachment: | 12343.patch added |
---|
comment:4 Changed 10 years ago by
Authors: | → Luis Felipe Tabera Alonso |
---|---|
Reviewers: | → André Apitzsch |
Status: | needs_work → 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
Milestone: | sage-5.3 → sage-5.2 |
---|
comment:6 Changed 10 years ago by
I wanted to check the compiled docs before setting it ready for review but you were faster :)
Changed 10 years ago by
Attachment: | 12343_rebased.patch added |
---|
comment:7 Changed 10 years ago by
Description: | modified (diff) |
---|---|
Merged in: | → sage-5.2.rc0 |
Resolution: | → fixed |
Status: | positive_review → closed |
André, Mariah, since you were involved in #10623, whould you mind reviewing the ticket?