#10540: Spec and patches for singular affine toric varieties / algebraic schemes
----------------------------------+-----------------------------------------
Reporter: vbraun | Owner: AlexGhitza
Type: enhancement | Status: needs_review
Priority: major | Milestone: sage-4.7.1
Component: algebraic geometry | Keywords:
Work_issues: | Upstream: N/A
Reviewer: Andrey Novoseltsev | Author: Volker Braun
Merged: | Dependencies: #9918, #10525, #10023,
#10529
----------------------------------+-----------------------------------------
Comment(by novoselt):
Looks good!
For the second patch:
5. (line 433) The implementation looks very confusing to me: why would
you store in the field `embedding_morphism` an exception??? I would expect
either returning the stored value if it exists, or constructing the
default embedding into the ambient space...
1. (line 441) Am I right that `embedding_origin` may be a non-origin as
in not (0,0,0)? In this case I kind of really don't like using the word
`origin`, how about `marked_point` instead or `embegging_marking` or
`embedding_center` to put it next to the embedding?
1. (line 1847) There should be no "local", this scheme is just isomorphic
to a neighbourhood of the point.
1. (line 1859) {{{``result.embedding_morphism``}}} should be more like
{{{:meth:`embedding_morphism`}}} or at least
{{{``result.embedding_morphism()``}}}, same for (1862).
1. (line 2076) `affine_patch` was used for compatibility with projective
spaces, do we really need to change its name? If yes, then a) the old name
should be kept as well for compatibility, b) how about `covering_patch`
since toric varieties are printed as `covered by 5 affine patches`?
1. "algebraic" attached to patches seems a bit weird to me...
1. Aha, now I see that `affine_patch` is preserved. Now the question is -
do we really need three functions or maybe `affine_patch` with several
forms of input will do?..
1. (line 2169) Why 0 is listed as one of the ideal generators?
1. (line 2193) You have actually added a possibility to construct lattice
polytopes from a list of points, without packing them into a matrix and
then transposing ;-)
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/10540#comment:9>
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.
