Opened 12 years ago
Last modified 8 years ago
#9902 needs_info defect
base_extend() not implemented in MPolynomialRing
Reported by: | Volker Braun | Owned by: | Martin Albrecht |
---|---|---|---|
Priority: | major | Milestone: | sage-6.4 |
Component: | commutative algebra | Keywords: | |
Cc: | Andrey Novoseltsev, Niles Johnson | Merged in: | |
Authors: | Volker Braun | Reviewers: | |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
The base class Ring
defines base_extend()
, but the implementation needs to be overridden in the derived class MPolynomialRing
:
sage: sage: P.<x,y,z> = PolynomialRing(QQ,'x, y, z'); P Multivariate Polynomial Ring in x, y, z over Rational Field sage: P.base_extend(CC) --------------------------------------------------------------------------- TypeError Traceback (most recent call last) /home/vbraun/opt/sage-4.5.3/devel/sage-main/<ipython console> in <module>() /home/vbraun/Sage/sage/local/lib/python2.6/site-packages/sage/rings/ring.so in sage.rings.ring.Ring.base_extend (sage/rings/ring.c:3190)() TypeError: no base extension defined
The patch implements the override and adds documentation.
Attachments (1)
Change History (8)
Changed 12 years ago by
Attachment: | trax_9902_fix_base_extension.patch added |
---|
comment:1 Changed 12 years ago by
Status: | new → needs_review |
---|
comment:2 Changed 12 years ago by
Cc: | Andrey Novoseltsev Niles Johnson added |
---|
Andrey, I wrote this patch a while a go to be able to extend the base field of the toric coordinate ring. It might be useful...
comment:3 Changed 12 years ago by
Status: | needs_review → needs_info |
---|
I am not quite sure it is the right approach. It seems to me that we have two methods: change_ring
that constructs "the same object but over different ring" and base_extend
which does the same, but only if there is a natural coercion. Given this description, it seems to me that there should be only one implementation of base_extend
in the base class:
def base_extend(self, R): if R.has_coerce_map(self.base_ring()): return self.change_ring(R) else: raise TypeError("%s cannot be extened to %s!" % (self.base_ring(), R))
and then each derived class should implement change_ring
only. (If the detailed error message breaks a lot of doctests I am fine with keeping the current one.) Thoughts?
There is also discrepancy between actual argument names and their description in documentation (base_ring
vs. R
).
comment:4 Changed 9 years ago by
Milestone: | sage-5.11 → sage-5.12 |
---|
comment:5 Changed 9 years ago by
Milestone: | sage-6.1 → sage-6.2 |
---|
comment:6 Changed 9 years ago by
Milestone: | sage-6.2 → sage-6.3 |
---|
comment:7 Changed 8 years ago by
Milestone: | sage-6.3 → sage-6.4 |
---|
Initial patch