Opened 10 years ago

Closed 10 years ago

#8993 closed enhancement (fixed)

Representation of polynomial quotient rings in Singular

Reported by: SimonKing Owned by: was
Priority: major Milestone: sage-4.5.2
Component: interfaces Keywords: polynomial quotient ring, singular
Cc: Merged in: sage-4.5.2.alpha1
Authors: Simon King Reviewers: Martin Albrecht
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

Currently there is no representation of univariate polynomial quotient rings in the Singular interface; it was only implemented for the multivariate case.

The attached patch implements it:

sage: P.<x> = QQ[]
sage: Q.<xi> = P.quo([(x^2+1)])
sage: singular(xi)
xi
sage: singular(Q)
//   characteristic : 0
//   number of vars : 1
//        block   1 : ordering lp
//                  : names    xi
//        block   2 : ordering C
// quotient ring from ideal
_[1]=xi^2+1
sage: (singular(xi)*singular(xi)).NF('std(0)')
-1

Attachments (2)

8993_poly_quotient_in_singular.patch (2.8 KB) - added by SimonKing 10 years ago.
Implement polynomial quotient rings in singular interface
8993_poly_quotient_in_singular.p1.patch (1.2 KB) - added by SimonKing 10 years ago.
Avoid "import sage" according to the reviewer's request - apply after first patch

Download all attachments as: .zip

Change History (12)

comment:1 Changed 10 years ago by SimonKing

  • Status changed from new to needs_review

comment:2 follow-up: Changed 10 years ago by robertwb

  • Status changed from needs_review to needs_work

Mostly looks good, the only issue is that you should never "import sage" (or sage.all) from within the sage library.

comment:3 in reply to: ↑ 2 ; follow-up: Changed 10 years ago by SimonKing

Replying to robertwb:

Mostly looks good, the only issue is that you should never "import sage" (or sage.all) from within the sage library.

Do I understand correctly:

It is OK that I do from sage.all import singular inside one method, but I should not do import sage on top of the file?

I'll submit a correction soon.

Cheers,

Simon

comment:4 Changed 10 years ago by robertwb

Yes.

comment:5 in reply to: ↑ 3 Changed 10 years ago by SimonKing

  • Status changed from needs_work to needs_review

Replying to SimonKing:

... It is OK that I do from sage.all import singular inside one method, but I should not do import sage on top of the file?

I'll submit a correction soon.

Done!

comment:6 Changed 10 years ago by malb

  • Status changed from needs_review to positive_review

Patch looks okay, doctests pass.

comment:7 follow-up: Changed 10 years ago by ddrake

  • Reviewers set to Martin Albrecht
  • Status changed from positive_review to needs_work

Please include the ticket number in the commit messages for your patches!

Changed 10 years ago by SimonKing

Implement polynomial quotient rings in singular interface

Changed 10 years ago by SimonKing

Avoid "import sage" according to the reviewer's request - apply after first patch

comment:8 in reply to: ↑ 7 ; follow-up: Changed 10 years ago by SimonKing

  • Status changed from needs_work to positive_review

Replying to ddrake:

Please include the ticket number in the commit messages for your patches!

I changed the commit message accordingly and updated the attachments. I hope this entitles me to switch back to "positive review".

comment:9 in reply to: ↑ 8 Changed 10 years ago by ddrake

Replying to SimonKing:

I changed the commit message accordingly and updated the attachments. I hope this entitles me to switch back to "positive review".

It certainly does. Thanks for fixing this! It should get merged in 4.5.2.alpha1.

comment:10 Changed 10 years ago by ddrake

  • Merged in set to sage-4.5.2.alpha1
  • Resolution set to fixed
  • Status changed from positive_review to closed

Merged both patches in 4.5.2.alpha1.

Note: See TracTickets for help on using tickets.