Opened 10 years ago
Closed 10 years ago
#14120 closed enhancement (fixed)
Add constant_coefficient method for Laurent polynomials
Reported by: | Travis Scrimshaw | Owned by: | Travis Scrimshaw |
---|---|---|---|
Priority: | major | Milestone: | sage-5.8 |
Component: | algebra | Keywords: | days45 |
Cc: | Merged in: | sage-5.8.beta2 | |
Authors: | Travis Scrimshaw | Reviewers: | Kannappan Sampath |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
There is no (easy) way to obtain the coefficient on the constant term for an element in the Laurent polynomial ring. This just adds a method constant_coefficient()
.
Attachments (1)
Change History (7)
comment:1 Changed 10 years ago by
Status: | new → needs_review |
---|
comment:2 Changed 10 years ago by
comment:3 follow-up: 4 Changed 10 years ago by
Reviewers: | → Kannappan Sampath |
---|---|
Status: | needs_review → needs_work |
I thought, I'd review this ticket.
The docstring claims that, if the number of inputs is less than the number of variables, all the remaining[trailing] variables would have their exponent set to zero. But, for some reason, the implementation (and hence the examples) raises a TypeError
.
IMHO, it would be nice to implement the behaviour suggested by the docstring. (of course, the other case, where no. of variables <= no. of inputs is being handled correctly.)
And, could we also please document that multivariate laurent polynomials are not iterable, as implemented, by adding an example, before we get complaints "like" this.
Building documentation, there is a warning, which is fairly easy to fix: I think the complaint is about the method __call__
<autodoc>:0: WARNING: Block quote ends without a blank line; unexpected unindent.
Finally, that we are changing this file, one might as well go ahead and clean up all the codifications: self is almost never codified, for instance and such documentation stuff...
The already extant method cofficient()
would benefit from adding r: r"""
just so that the \frac
with backslash is picked up properly by Sphinx.
Regards, ~KnS
Changed 10 years ago by
Attachment: | trac_14120-constant_coeff_laurent_poly-ts.patch added |
---|
comment:4 Changed 10 years ago by
Status: | needs_work → needs_review |
---|
First off, thank you for doing the review.
Replying to knsam:
The docstring claims that, if the number of inputs is less than the number of variables, all the remaining[trailing] variables would have their exponent set to zero. But, for some reason, the implementation (and hence the examples) raises a
TypeError
.IMHO, it would be nice to implement the behaviour suggested by the docstring. (of course, the other case, where no. of variables <= no. of inputs is being handled correctly.)
I made it consistent with other multivariate polynomial rings, however I forgot to correct the doc. This is done. I've also added in two more methods for consistancy and changed the output of the iterator.
And, could we also please document that multivariate laurent polynomials are not iterable, as implemented, by adding an example, before we get complaints "like" this.
This does not make sense to me. They are iterable with this patch.
Building documentation, there is a warning, which is fairly easy to fix: I think the complaint is about the method
__call__
<autodoc>:0: WARNING: Block quote ends without a blank line; unexpected unindent.
I do not get this with the revised patch.
Finally, that we are changing this file, one might as well go ahead and clean up all the codifications: self is almost never codified, for instance and such documentation stuff...
You are welcome to do so on another ticket. That is outside of the purview of this one. I did it in the methods this patch touches.
The already extant method
cofficient()
would benefit from adding r:r"""
just so that the\frac
with backslash is picked up properly by Sphinx. There is a misspelt word in the Note that is already there too. (constnat...)
Fixed.
Best, Travis
comment:5 Changed 10 years ago by
Status: | needs_review → positive_review |
---|
And, could we also please document that multivariate laurent polynomials are not iterable, as implemented, by adding an example, before we get complaints "like" this.
This does not make sense to me. They are iterable with this patch.
I meant more along the following lines: you cannot iterate through using slice notation:
sage: P.<x,y,z> = LaurentPolynomialRing(QQ) sage: f = (y^2 - x^9 - 7*x*y^3 + 5*x*y)*x^-3 + x*z; f -x^6 + x*z - 7*x^-2*y^3 + 5*x^-2*y + x^-3*y^2 sage: f[6,0,0] -1 sage:f[5:6] [snip] TypeError: Multivariate Laurent polynomials are not iterable
But, well, this does not even make sense, given the current implementation. So, I'll set this to positive review.
~KnS
comment:6 Changed 10 years ago by
Merged in: | → sage-5.8.beta2 |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
Fixed doctests and added a general
__getitem__()
to retrieve the coefficient by exponent value: