#9713: Add toric Chow group
----------------------------------------------+-----------------------------
Reporter: vbraun | Owner: AlexGhitza
Type: enhancement | Status: needs_work
Priority: major | Milestone: sage-4.6.2
Component: algebraic geometry | Keywords:
Author: Volker Braun | Upstream: N/A
Reviewer: Andrey Novoseltsev, Simon King | Merged:
Work_issues: element class |
----------------------------------------------+-----------------------------
Changes (by SimonKing):
* status: needs_review => needs_work
* reviewer: Andrey Novoseltsev => Andrey Novoseltsev, Simon King
* work_issues: => element class
Comment:
I applied the patches on top of `sage-4.6.1.alpha3`, and the good news is
that all tests pass.
However, I have some criticism concerning `trac_9713_fix_fg_pid.patch`.
In several cases I see things like
{{{
return self.parent().Element(self.parent(), self._x - other._x)
}}}
That's not how the attribute `Element` is supposed to be used. As far as I
know, the story is like this:
* One provides an attribute `Element`, which is supposed to be a class.
* `Parent.element_class` is a so-called lazy attribute. That's to say, if
one does `self.element_class` then the ''method'' `self.element_class()`
is called, which computes a ''new'' class out of `self.Element` and
`self.category()`, assigns this new class to an actual (not lazy)
attribute `self.element_class`, and returns that new class.
* When one requests `self.element_class` is called the next time then it
is already a usual attribute, which is of course much faster than calling
a method.
Hence, you should define `self.Element`. But then you should do
{{{
P = self.parent()
return P.element_class(P, self._x - other._x)
}}}
since `P.element_class` (in contrast to `P.Element`) also inherits stuff
from the category. Note that additionally the code snipped spares a little
time compared with your patch since `self.parent()` is called only once
instead of twice.
As I mentioned, my bandwith is a bit restricted at the moment. So, this
post might be incomplete, it is just what I found by a little reading of
the patch.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/9713#comment:48>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
