Opened 13 years ago
Closed 13 years ago
#6590 closed defect (fixed)
[with patch, positive review] Cython __new__ should be __cinit__
Reported by: | Robert Bradshaw | Owned by: | tbd |
---|---|---|---|
Priority: | major | Milestone: | sage-4.1.2 |
Component: | build | Keywords: | |
Cc: | Merged in: | Sage 4.1.2.alpha0 | |
Authors: | Robert Bradshaw | Reviewers: | Alex Ghitza |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
This changed a while back, but as long as the old form is in the library we won't be able to really implement a (Python-style) __new__
and also people will keep using it in new code by analogy.
Attachments (2)
Change History (6)
Changed 13 years ago by
Attachment: | 6590-cinit.patch added |
---|
comment:1 Changed 13 years ago by
comment:2 Changed 13 years ago by
If no init parameters are not needed by cinit they can simply be omitted. This saves on argument-parsing code, especially as kwds needs to construct an empty dictionary each time. Also note that cinit is called on PY_NEW, so the savings here is really nice. (Essentially, __cinit__(self)
is implicitly __cinit__(self, *args, **kwds)
where *args and kwds are not parsed because they're not needed.
Changed 13 years ago by
Attachment: | 6590-cinit_rebased.patch added |
---|
rebased against sage-4.1.1, apply instead of previous patch
comment:3 Changed 13 years ago by
Authors: | → Robert Bradshaw |
---|---|
Reviewers: | → Alex Ghitza |
Summary: | [with patch, needs review] Cython __new__ should be __cinit__ → [with patch, positive review] Cython __new__ should be __cinit__ |
Positive review. I had to rebase it against sage-4.1.1 since it didn't apply cleanly.
comment:4 Changed 13 years ago by
Merged in: | → Sage 4.1.2.alpha0 |
---|---|
Resolution: | → fixed |
Status: | new → closed |
Merged 6590-cinit_rebased.patch
.
How come the some of the new cinit functions have different signatures from the corresponding init functions? I thought the signatures should be the same, or at least the cinit should have a *args or kwds to accept the arguments passed to init
In particular, I refer to sage/libs/ntl/ntl_mat_GF2.pyx, sage/libs/ntl/ntl_mat_ZZ.pyx, sage/matrix/matrix_integer_2x2.pyx, etc.