Opened 23 months ago
Closed 23 months ago
#24607 closed enhancement (fixed)
Add abstract _add_ and _mul_ methods to RingElement
Reported by: | jdemeyer | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.2 |
Component: | cython | Keywords: | |
Cc: | tscrim | Merged in: | |
Authors: | Jeroen Demeyer | Reviewers: | Travis Scrimshaw |
Report Upstream: | N/A | Work issues: | |
Branch: | d984df5 (Commits) | Commit: | d984df507be6b352f87bafbc388e23d7e5995031 |
Dependencies: | Stopgaps: |
Description (last modified by )
In order to fix all Compatible but non-identical C method '_add_' not redeclared in definition part of extension type '...'. This may cause incorrect vtables to be generated.
warnings, we add abstract methods _add_
and _mul_
to some base Element
classes.
This is part of #23600.
In #24116, such abstract methods were added to the finite ring element base class. Those can be removed again.
Change History (14)
comment:1 Changed 23 months ago by
- Cc tscrim added
- Description modified (diff)
comment:2 Changed 23 months ago by
- Description modified (diff)
comment:3 Changed 23 months ago by
- Branch set to u/jdemeyer/add_abstract__add__and__mul__methods_to_ringelement
comment:4 Changed 23 months ago by
- Commit set to 8180e22ae94307e70927d198edf7c351397a7d73
- Status changed from new to needs_review
comment:5 Changed 23 months ago by
I know it doesn't really matter, but the error string for the NotImplementedError
I think is not formatted properly (missing the leading f
).
comment:6 Changed 23 months ago by
- Reviewers set to Travis Scrimshaw
Otherwise LGTM and you can set a positive review on my behalf either way.
comment:7 Changed 23 months ago by
Ooops...
comment:8 Changed 23 months ago by
- Commit changed from 8180e22ae94307e70927d198edf7c351397a7d73 to 4715809a8aaf336a92775902e747a69bb1505a67
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
4715809 | Add abstract _add_ and _mul_ methods
|
comment:10 Changed 23 months ago by
- Status changed from positive_review to needs_work
There are some unexpected doctest failures.
comment:11 Changed 23 months ago by
- Commit changed from 4715809a8aaf336a92775902e747a69bb1505a67 to d984df507be6b352f87bafbc388e23d7e5995031
Branch pushed to git repo; I updated commit sha1. New commits:
d984df5 | Fix GapElement_generic._add_
|
comment:12 Changed 23 months ago by
- Status changed from needs_work to needs_review
comment:13 Changed 23 months ago by
- Status changed from needs_review to positive_review
Tests now pass.
comment:14 Changed 23 months ago by
- Branch changed from u/jdemeyer/add_abstract__add__and__mul__methods_to_ringelement to d984df507be6b352f87bafbc388e23d7e5995031
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
Add abstract _add_ and _mul_ methods