Opened 10 years ago
Closed 9 years ago
#14120 closed enhancement (fixed)
Add constant_coefficient method for Laurent polynomials
Reported by: | tscrim | Owned by: | tscrim |
---|---|---|---|
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 changed from new to needs_review
comment:2 Changed 10 years ago by
comment:3 follow-up: ↓ 4 Changed 9 years ago by
- Reviewers set to Kannappan Sampath
- Status changed from needs_review to 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. There is a misspelt word in the Note that is already there too. (constnat...)
Regards, ~KnS
Changed 9 years ago by
comment:4 in reply to: ↑ 3 Changed 9 years ago by
- Status changed from needs_work to 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 9 years ago by
- Status changed from needs_review to 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 9 years ago by
- Merged in set to sage-5.8.beta2
- Resolution set to fixed
- Status changed from positive_review to closed
Fixed doctests and added a general
__getitem__()
to retrieve the coefficient by exponent value: