Opened 10 years ago
Closed 8 years ago
#13442 closed enhancement (fixed)
provide gcd for new polynomial rings through _gcd_univariate_polynomial
Reported by: | saraedum | Owned by: | AlexGhitza |
---|---|---|---|
Priority: | minor | Milestone: | sage-6.4 |
Component: | basic arithmetic | Keywords: | sd59 |
Cc: | Merged in: | ||
Authors: | Julian Rueth | Reviewers: | Bruno Grenet |
Report Upstream: | N/A | Work issues: | |
Branch: | 8126ef1 (Commits, GitHub, GitLab) | Commit: | 8126ef15551e8d8ed5d0edb2d559f5a9fd87bcf1 |
Dependencies: | #13441 | Stopgaps: |
Description
Currently, to add gcd functionality for a new polynomial ring, one needs to add a specialized subclass of PolynomialElement
.
The attached patch allows rings to provide a _gcd_univariate_polynomial
method which will be called by PolynomialElement
to compute gcds.
This is similar to #10635.
Attachments (1)
Change History (19)
comment:1 Changed 10 years ago by
- Dependencies changed from 13441 to #13441
Changed 10 years ago by
comment:2 Changed 10 years ago by
- Status changed from new to needs_review
comment:3 follow-up: ↓ 5 Changed 10 years ago by
- Status changed from needs_review to needs_work
comment:4 Changed 9 years ago by
- Milestone changed from sage-5.11 to sage-5.12
comment:5 in reply to: ↑ 3 Changed 9 years ago by
Replying to saraedum:
I want to remove
self
from the docstrings.
Is this part of some style recommendation? I would say referring to self
in the docstring is perfectly acceptable, since there is an object called self
and it is usually clear what kind of object it is. Personally, I strongly prefer self
over constructions like "this element".
comment:6 Changed 9 years ago by
- Branch set to u/niles/ticket/13442
- Created changed from 09/09/12 09:28:03 to 09/09/12 09:28:03
- Modified changed from 08/13/13 19:34:26 to 08/13/13 19:34:26
comment:7 Changed 9 years ago by
- Commit set to b3eee8a1f30a328eeb14080f07dee9ae26ab0ec5
rebased and converted to git branch; no other changes
New commits:
b3eee8a | Trac #13442: rings can provide _gcd_univariate_polynomial for polynomial factorization
|
comment:8 Changed 9 years ago by
- Milestone changed from sage-6.1 to sage-6.2
comment:9 Changed 8 years ago by
- Milestone changed from sage-6.2 to sage-6.3
comment:10 Changed 8 years ago by
- Branch changed from u/niles/ticket/13442 to u/saraedum/ticket/13442
- Modified changed from 05/06/14 15:20:58 to 05/06/14 15:20:58
comment:11 Changed 8 years ago by
- Keywords sd59 added
- Status changed from needs_work to needs_review
comment:12 Changed 8 years ago by
- Commit changed from b3eee8a1f30a328eeb14080f07dee9ae26ab0ec5 to 2947606572e5421301eb408d26066def13b267d4
comment:13 Changed 8 years ago by
- Commit changed from 2947606572e5421301eb408d26066def13b267d4 to 8126ef15551e8d8ed5d0edb2d559f5a9fd87bcf1
Branch pushed to git repo; I updated commit sha1. New commits:
8126ef1 | Merge branch 'develop' into ticket/13442
|
comment:14 Changed 8 years ago by
- Milestone changed from sage-6.3 to sage-6.4
comment:15 follow-up: ↓ 16 Changed 8 years ago by
I agree with pbruin on self
versus this polynomial
. I find the documentation clearer with self
, and this seems to be more consistent with the rest of Sage.
Tell me what do you think of this, I'll positive review the ticket then.
comment:16 in reply to: ↑ 15 Changed 8 years ago by
Replying to bruno:
I agree with pbruin on
self
versusthis polynomial
. I find the documentation clearer withself
, and this seems to be more consistent with the rest of Sage.
Both approaches have their drawbacks. self
makes it harder to read if you do not know about python, i.e., if a 'user' consults the help. 'this polynomial' makes things slightly more difficult to understand if you know about self
.
I have been asked on different tickets to replace self
with something more appropriate. Sage is not really consistent with this.
Tell me what do you think of this, I'll positive review the ticket then. All tests passed with the current commits.
I do not really care how we do this in sage. Either way is fine with me.
comment:17 Changed 8 years ago by
- Reviewers set to Bruno Grenet
- Status changed from needs_review to positive_review
I actually haven't a strong case for or against any of both solutions. I could have set it to positive review earlier, so let me do it now!
comment:18 Changed 8 years ago by
- Branch changed from u/saraedum/ticket/13442 to 8126ef15551e8d8ed5d0edb2d559f5a9fd87bcf1
- Resolution set to fixed
- Status changed from positive_review to closed
I want to remove
self
from the docstrings.