#9972: Add toric lattice morphisms
-------------------------------------+--------------------------------------
Reporter: novoselt | Owner: mhampton
Type: enhancement | Status: needs_work
Priority: major | Milestone: sage-4.6
Component: geometry | Keywords:
Author: Andrey Novoseltsev | Upstream: N/A
Reviewer: Volker Braun | Merged:
Work_issues: switch to FanMorphism |
-------------------------------------+--------------------------------------
Comment(by novoselt):
I meant that mistake would be realized quite fast once the code was
actually used. I didn't actually use code for toric divisors yet,
especially with cones from another fan. But anyway - this is all
hypothetical and I may be very wrong.
My main point is not that it is difficult to replace 13 uses of
`ambient_ray_indices`, but that it is a very convenient and natural
function which I personally use both when developing new code and when
actually using Sage interactively. I would rather not pass any arguments
to it, because it is annoying for interaction and in functions it may lead
to code like
{{{
cone.ambient_ray_indices(cone.ambient())
}}}
or
{{{
cone.ambient().ray_indices(cone)
}}}
which is plain confusing.
The name of the function clearly indicates that there is something ambient
and cone must be aware of it, since no arguments were passed. Therefore,
when using this function, the user should be sure that this something
ambient is what it is supposed to be. In fact, it may be more clear for
new outside users than for us, since we are used to writing code inside
the class where in many cases this requirement is definitely satisfied.
(How can `self` have a wrong `self.ambient()`???) And after all this
discussion I will probably remember very well for a long time to make an
extra check that this function is used after making sure that ambient is
what it is assumed to be.
If you want to add extra functionality like `fan.ray_indices(cone)` which
will be safer to use and then use this one when you want - I am fine with
it. But if you insist on getting rid of `cone.ambient_ray_indices()` in
its present form/namespace, we need to invite more people from sage-devel
for opinions...
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/9972#comment:29>
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.
