Opened 15 years ago
Closed 14 years ago
#1440 closed defect (fixed)
[with patch, with positive review] Inconsistency in subs and substitute for univariate polynomials
Reported by: | was | Owned by: | broune |
---|---|---|---|
Priority: | major | Milestone: | sage-3.0.3 |
Component: | basic arithmetic | Keywords: | |
Cc: | Merged in: | ||
Authors: | Reviewers: | ||
Report Upstream: | Work issues: | ||
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
I very much agree that the problem below is a bug, which we should resolve.
subs and substitute are not equivalent for single variable polynomials, though they are in the field of fractions or for polynomials in more than one variable: ---------------------------------------------------------------------- | SAGE Version 2.8.15, Release Date: 2007-12-03 | | Type notebook() for the GUI, and license() for information. | ---------------------------------------------------------------------- sage: R.<x> = QQ[] sage: f = x^3 + x - 3 sage: f.subs(x = 5) 127 sage: f.substitute(x = 5) --------------------------------------------------------------------------- <type 'exceptions.ValueError'> Traceback (most recent call last) /Users/mafwc/<ipython console> in <module>() /Users/mafwc/element.pyx in sage.structure.element.Element.substitute() /Users/mafwc/polynomial_element.pyx in sage.rings.polynomial.polynomial_element.Polynomial.subs() /Users/mafwc/polynomial_element.pyx in sage.rings.polynomial.polynomial_element.Polynomial.__call__() <type 'exceptions.ValueError'>: must not specify both a keyword and positional argument sage: g = f/(x - 1) sage: [g.subs(x = 5), g.substitute(x = 5)] [127/4, 127/4] sage: R2.<y, z> = PolynomialRing(QQ, 2) sage: h = y^3*z + 4*y*z^2 + y + 3*z - 7 sage: [h.subs(y = 5), h.substitute(y = 5)] [20*z^2 + 128*z - 2, 20*z^2 + 128*z - 2] [Mac OS X 10.4.11, 2 GHz Intel Core 2 Duo, 1 GB].
Attachments (2)
Change History (12)
comment:1 Changed 15 years ago by
comment:2 Changed 14 years ago by
- Owner changed from somebody to broune
- Summary changed from Inconsistency in subs and substitute for univariate polynomials to [with patch, needs review] Inconsistency in subs and substitute for univariate polynomials
comment:3 Changed 14 years ago by
- Status changed from new to assigned
Changed 14 years ago by
comment:4 Changed 14 years ago by
- Summary changed from [with patch, needs review] Inconsistency in subs and substitute for univariate polynomials to [with patch, with partial review] Inconsistency in subs and substitute for univariate polynomials
Review and questions:
If this is just an alias would it not be simpler to just put
substitute = subs
in the class definition instead of defining a second function which calls the first?
Secondly, for some reason the patch posted does not display in the normal way, but I don't know why.
I don't really know enough about the issues raised by jbmohler to judge this solution; but if this does solve the problem then the simple assignment I suggested would also!
comment:5 follow-up: ↓ 6 Changed 14 years ago by
Is it possible to attach a docstring if the function is written using an assignment? Otherwise you would get no docstring, or possibly the docstring for subs, upon calling help on subtitute.
comment:6 in reply to: ↑ 5 Changed 14 years ago by
Replying to broune:
Is it possible to attach a docstring if the function is written using an assignment? Otherwise you would get no docstring, or possibly the docstring for subs, upon calling help on subtitute.
Not literally, but after the assignment they are the same function so the docstring for one is displayed for both. For example, prime_factors
is a synonym for prime_divisors
, and look:
sage: n=123 sage: n.prime_factors? Type: builtin_function_or_method Base Class: <type 'builtin_function_or_method'> String Form: <built-in method prime_divisors of sage.rings.integer.Integer object at 0xa124980> Namespace: Interactive Docstring: The prime divisors of self, sorted in increasing order. If n is negative, we do *not* include -1 among the prime divisors, since -1 is not a prime number. EXAMPLES: sage: a = 1; a.prime_divisors() [] sage: a = 100; a.prime_divisors() [2, 5] sage: a = -100; a.prime_divisors() [2, 5] sage: a = 2004; a.prime_divisors() [2, 3, 167]
Although that looks slightly strange, I think it is ok, especially if the docstring mentions the synonym (which in this case it does not, which is my fault since I inserted the synonym while no-one was looking).
comment:7 Changed 14 years ago by
The new patch uses = to redefine substitute. I would add a doctest, but I don't know where it would go. It does make the code in the ticket work. Either version is fine by me.
comment:8 Changed 14 years ago by
- Summary changed from [with patch, with partial review] Inconsistency in subs and substitute for univariate polynomials to [with patch, with positive review] Inconsistency in subs and substitute for univariate polynomials
I can see no reason not to apply this one-liner (redef_substi2.patch), even if there are other related issues which the patch does not resolve.
comment:9 Changed 14 years ago by
I second that, please apply. I'll open a new ticket for:
This is somewhat tangential to this issue, but subs has been overridden in a few different classes and the overridden implementation is not quite as robust as (or duplicates code from) the implementation in the element base class. I think that the base implementation/architecture should be strengthened in ways that make these overrides unneeded. Of course, then the subs/substitute synonym should entirely be handled by the base class making it impossible for the inconsistency of the noted bug.
comment:10 Changed 14 years ago by
- Resolution set to fixed
- Status changed from assigned to closed
Merged redef_substi2.patch in Sage 3.0.3.alpha1
This is somewhat tangential to this issue, but subs has been overridden in a few different classes and the overridden implementation is not quite as robust as (or duplicates code from) the implementation in the element base class. I think that the base implementation/architecture should be strengthened in ways that make these overrides unneeded. Of course, then the subs/substitute synonym should entirely be handled by the base class making it impossible for the inconsistency of the noted bug.
Note that because of the issues I mention in the first paragraph I highly suspect that this bug is not unique to univ-variate polys.