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 )
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