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: |
Description (last modified by )
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)
Change History (15)
Changed 10 years ago by
Attachment: | trac14818-pari_FF_decl.patch added |
---|
comment:1 Changed 10 years ago by
Description: | modified (diff) |
---|---|
Status: | new → needs_review |
comment:2 Changed 10 years ago by
Cc: | jpflori added |
---|
comment:3 Changed 10 years ago by
Reviewers: | → Jean-Pierre Flori |
---|---|
Status: | needs_review → needs_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
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
Work issues: | doctests → doctests, docstring |
---|
comment:6 Changed 10 years ago by
Description: | modified (diff) |
---|---|
Status: | needs_work → positive_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
Description: | modified (diff) |
---|
comment:11 Changed 10 years ago by
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
Merged in: | → sage-5.12.beta0 |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
based on 5.11.beta3