Opened 6 years ago

Closed 6 years ago

#14957 closed enhancement (fixed)

Square-free decomposition and any_root for polynomials

Reported by: pbruin Owned by:
Priority: major Milestone: sage-5.13
Component: algebra Keywords: polynomial sd51
Cc: roed, jpflori, mraum, fstromberg, JCooley, davidloeffler, dfesti Merged in: sage-5.13.beta0
Authors: David Roe, Jean-Pierre Flori, Peter Bruin Reviewers: Jean-Pierre Flori, Peter Bruin
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #14888 Stopgaps:

Description (last modified by jpflori)

This patch improves the method Polynomial.squarefree_decomposition() and adds a new method Polynomial.any_root(). This has been split off from #8335, and is a dependency of that ticket.

Apply:

Attachments (3)

squarefree_decomposition_any_root.patch (15.4 KB) - added by pbruin 6 years ago.
enhance Polynomial.squarefree_decomposition(), add Polynomial.any_root()
trac_14957-squarefree_decomposition_any_root.patch (16.2 KB) - added by pbruin 6 years ago.
enhance Polynomial.squarefree_decomposition(), add Polynomial.any_root()
trac_14957-doctests_14888.patch (1.6 KB) - added by jpflori 6 years ago.
Rebase doctests on top of #14888

Download all attachments as: .zip

Change History (15)

Changed 6 years ago by pbruin

enhance Polynomial.squarefree_decomposition(), add Polynomial.any_root()

comment:1 Changed 6 years ago by pbruin

  • Keywords sd51 added
  • Status changed from new to needs_review

comment:2 Changed 6 years ago by pbruin

  • Cc roed jpflori mraum fstromberg JCooley davidloeffler dfesti added

CC-ing the authors of #8335 and Sage Days 51 participants on this split-off ticket.

Changed 6 years ago by pbruin

enhance Polynomial.squarefree_decomposition(), add Polynomial.any_root()

comment:3 Changed 6 years ago by pbruin

  • Description modified (diff)

New patch with various improvements. Most importantly: improved documentation, removed unused parameter extend of any_root, which caused a doctest failure.

Patchbot: apply trac_14957-squarefree_decomposition_any_root.patch

comment:4 follow-up: Changed 6 years ago by jpflori

  • Authors changed from David Roe, Jean-Pierre Flori to David Roe, Jean-Pierre Flori, Peter Bruin
  • Reviewers set to Jean-Pierre Flori, Peter Bruin
  • Status changed from needs_review to positive_review

I already reviewed David's code and did not really touch this part apart for a few fixes.

I also agree with Peter's changes.

Everything seems to work fine, but note that if I test this on top of #14888, then two doctests fail. I think we should make this one depend on #14888, does anyone disagree? If we go this way, this should get positive review.

comment:5 Changed 6 years ago by jpflori

  • Status changed from positive_review to needs_info
  • Work issues set to rebase on top of #14888?

comment:6 in reply to: ↑ 4 Changed 6 years ago by pbruin

Replying to jpflori:

Everything seems to work fine, but note that if I test this on top of #14888, then two doctests fail. I think we should make this one depend on #14888, does anyone disagree?

That makes sense, since #14888 already has a positive review and we would have to change one or the other anyway.

comment:7 Changed 6 years ago by jpflori

  • Dependencies set to #14888
  • Description modified (diff)
  • Status changed from needs_info to positive_review
  • Work issues rebase on top of #14888? deleted
Patchbot: apply
* trac_14957-squarefree_decomposition_any_root.patch
* trac_14957-doctests_14888.patch
Last edited 6 years ago by jpflori (previous) (diff)

Changed 6 years ago by jpflori

Rebase doctests on top of #14888

comment:8 Changed 6 years ago by jpflori

  • Description modified (diff)

comment:9 Changed 6 years ago by pbruin

Those were also the doctest failures I was getting, so I am happy with the fixes and the positive review.

comment:10 Changed 6 years ago by jdemeyer

  • Milestone changed from sage-5.12 to sage-pending

comment:11 Changed 6 years ago by jdemeyer

  • Milestone changed from sage-pending to sage-5.13

comment:12 Changed 6 years ago by jdemeyer

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