Opened 10 years ago

Closed 10 years ago

#14818 closed enhancement (fixed)

Declare PARI finite field functions (FF_*), wrap ffgen() and ffinit()

Reported by: pbruin Owned by: cpernet
Priority: minor Milestone: sage-5.12
Component: finite rings Keywords: pari
Cc: jpflori Merged in: sage-5.12.beta0
Authors: Peter Bruin Reviewers: Jean-Pierre Flori
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by jpflori)

To access the PARI library functions related to the t_FFELT type for finite fields (see #12142), these functions need to be declared in sage/libs/pari/decl.pxi. Also, wrappers for the functions ffgen() and ffinit() are missing.

I am attaching a patch, which will become a dependency of #12142.

Apply:

Attachments (3)

trac14818-pari_FF_decl.patch (3.6 KB) - added by pbruin 10 years ago.
based on 5.11.beta3
trav_14818-reviewer.patch (2.6 KB) - added by jpflori 10 years ago.
Reviewer patch.
trac_14818-reviewer.patch (2.2 KB) - added by jpflori 10 years ago.
Reviewer patch.

Download all attachments as: .zip

Change History (15)

Changed 10 years ago by pbruin

based on 5.11.beta3

comment:1 Changed 10 years ago by pbruin

Description: modified (diff)
Status: newneeds_review

comment:2 Changed 10 years ago by jpflori

Cc: jpflori added

comment:3 Changed 10 years ago by jpflori

Reviewers: Jean-Pierre Flori
Status: needs_reviewneeds_work
Work issues: doctests

Please add some doctests.

I know this will be indirectly doctested through further tickets, but it would be nice to have some example directly in the docstrings of ffgen and ffinit.

comment:4 Changed 10 years ago by jpflori

There is also a problem with the docstring for ffinit which mentions a polynomial P.

And the doc avout the v variable is kind of obscure. In the docstring for get_var it's mentioned it should be a string or -1. Here it seems you suggest it can be an integer. Indeed, the PARI ffinit functions wants a long and will treat negative input in a special way, but the point of the call to get_var is to convert the var's string to the correct integer. So either skip the get_var call or correcte the docstring. I think the second option is better, and is what you wanted, isn't it?

comment:5 Changed 10 years ago by jpflori

Work issues: doctestsdoctests, docstring

Changed 10 years ago by jpflori

Attachment: trav_14818-reviewer.patch added

Reviewer patch.

comment:6 Changed 10 years ago by jpflori

Description: modified (diff)
Status: needs_workpositive_review
Work issues: doctests, docstring

Just added simple doctests and corrected the doc, so let's consider this a reviewer patch and I give positive review to the real changes made by Peter.

comment:7 Changed 10 years ago by jpflori

Description: modified (diff)

comment:8 Changed 10 years ago by pbruin

Thanks, I realise that I was really sloppy with the docstrings...

Changed 10 years ago by jpflori

Attachment: trac_14818-reviewer.patch added

Reviewer patch.

comment:9 Changed 10 years ago by jpflori

Description: modified (diff)

Correct and better named patch.

comment:10 Changed 10 years ago by jpflori

comment:11 Changed 10 years ago by pbruin

The patchbot doesn't seem to get it:

Apply trac14818-pari_FF_decl.patch, trac_14818-reviewer.patch

comment:12 Changed 10 years ago by jdemeyer

Merged in: sage-5.12.beta0
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.