Opened 6 years ago

Closed 6 years ago

#16886 closed enhancement (fixed)

Add PARI's idealchinese function to Sage

Reported by: mmasdeu Owned by:
Priority: minor Milestone: sage-6.4
Component: number fields Keywords: pari idealchinese crt
Cc: aurel Merged in:
Authors: Marc Masdeu Reviewers: Peter Bruin
Report Upstream: N/A Work issues:
Branch: aa6aea8 (Commits) Commit: aa6aea8c8a48c51306d135779f7fdbd5b0adc337
Dependencies: Stopgaps:


This ticket adds idealchinese from PARI.

Change History (7)

comment:1 Changed 6 years ago by mmasdeu

  • Branch set to u/mmasdeu/16886-pariidealchinese
  • Commit set to 12cf71a4b8a2dff5eaa5160a7e3dea818b28435b
  • Status changed from new to needs_review

New commits:

12cf71aTrac 16886: Added pari's idealchinese function.

comment:2 Changed 6 years ago by mmasdeu

  • Component changed from PLEASE CHANGE to number fields
  • Priority changed from major to minor

comment:3 Changed 6 years ago by pbruin

The code looks good to me. Here are just a few remarks about the documentation (sorry that this is longer than the documentation itself...):

  • The first line should be a short description of what the function does.
  • The usual format for documenting argument lists has -- instead of :, e.g.
    - ``x`` -- prime ideal factorization
  • A typo: elementbb -> element b
  • Use single backquotes around formulae so they are LaTeXed in the HTML and PDF documentation, and use LaTeX macros like \ge instead of >=.
  • Maybe it is useful to mention the ambient number field somewhere...

comment:4 Changed 6 years ago by git

  • Commit changed from 12cf71a4b8a2dff5eaa5160a7e3dea818b28435b to aa6aea8c8a48c51306d135779f7fdbd5b0adc337

Branch pushed to git repo; I updated commit sha1. New commits:

aa6aea8Trac 16886: fixed documentation, added one doctest.

comment:5 Changed 6 years ago by mmasdeu

I fixed the patch following your comments (thanks!).

comment:6 Changed 6 years ago by pbruin

  • Reviewers set to Peter Bruin
  • Status changed from needs_review to positive_review

comment:7 Changed 6 years ago by vbraun

  • Branch changed from u/mmasdeu/16886-pariidealchinese to aa6aea8c8a48c51306d135779f7fdbd5b0adc337
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.