Opened 11 years ago
Closed 11 years ago
#12062 closed enhancement (fixed)
FiniteField_ntl_gf2e to Python
Reported by: | roed | Owned by: | AlexGhitza |
---|---|---|---|
Priority: | major | Milestone: | sage-5.0 |
Component: | algebra | Keywords: | finite fields |
Cc: | Merged in: | sage-5.0.beta10 | |
Authors: | David Roe | Reviewers: | David Loeffler |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
Attachments (3)
Change History (14)
comment:1 Changed 11 years ago by
Status: | new → needs_review |
---|
Changed 11 years ago by
Attachment: | 12062.patch added |
---|
comment:2 Changed 11 years ago by
comment:3 Changed 11 years ago by
Reviewers: | → David Loeffler |
---|---|
Status: | needs_review → needs_work |
comment:4 Changed 11 years ago by
Thanks for all the reviews David! I'm at the Fields Institute this week for a conference, but will try to find some time to make changes.
comment:5 Changed 11 years ago by
Description: | modified (diff) |
---|
comment:6 Changed 11 years ago by
Description: | modified (diff) |
---|
Changed 11 years ago by
Attachment: | 12062_ref_changes.patch added |
---|
comment:7 Changed 11 years ago by
Status: | needs_work → needs_review |
---|
I've made the suggested changes, and created #12686 for adding sage.rings.finite_rings to the reference manual.
Changed 11 years ago by
Attachment: | 12062-final.patch added |
---|
Apply only this patch. Patch against 5.0.beta8
comment:8 Changed 11 years ago by
Description: | modified (diff) |
---|---|
Status: | needs_review → positive_review |
Hi David,
I'm happy with this: you've dealt with all of my concerns. Positive review. (Your second patch didn't have a commit message, so I qfolded it into the previous one, which also means we don't clutter the repository history unnecessarily; I didn't actually change anything in the code.)
comment:11 Changed 11 years ago by
Merged in: | → sage-5.0.beta10 |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
Note: See
TracTickets for help on using
tickets.
This is looks great, but here are some (relatively minor) gripes:
fetch_int
in the cython file has been rewritten and is now much more comprehensible, but the docstring for the same function in the new python file is still the old rather confusing one.finite_field_ntl_gf2e.py
to the reference manual? (This might be better off on a new ticket, since it makes no sense to have some backends and not others in the reference manual, and doing the others will be lots of work.)cdef class Cache_ntl_gf2e(SageObject):
would be nice, explaining briefly why the class exists.