Opened 9 years ago

Closed 9 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 9 years ago.
Implement polynomial quotient rings in singular interface
8993_poly_quotient_in_singular.p1.patch (1.2 KB) - added by SimonKing 9 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 9 years ago by SimonKing

  • Status changed from new to needs_review

comment:2 follow-up: Changed 9 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 9 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 9 years ago by robertwb

Yes.

comment:5 in reply to: ↑ 3 Changed 9 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 9 years ago by malb

  • Status changed from needs_review to positive_review

Patch looks okay, doctests pass.

comment:7 follow-up: Changed 9 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 9 years ago by SimonKing

Implement polynomial quotient rings in singular interface

Changed 9 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 9 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 9 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 9 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.