#9245: Add library of toric varieties
----------------------------------+-----------------------------------------
Reporter: vbraun | Owner: AlexGhitza
Type: enhancement | Status: needs_review
Priority: major | Milestone:
Component: algebraic geometry | Keywords:
Author: Volker Braun | Upstream: N/A
Reviewer: Andrey Novoseltsev | Merged:
Work_issues: |
----------------------------------+-----------------------------------------
Comment(by novoselt):
1) Can we replace C with A whenever it refers to complex numbers, e.g.
`C2` ===> `A2` with documentation "Return the affine plane `\mathbb{A}^2`
as a toric variety. ..."? I know that usually one thinks of complex
numbers in toric geometry (I do it always), but the default field is the
rational one and I think it would be better to have "neutral" names. Note
that there is already no reference to the base field in all projective
spaces, which is good, I think.
2) I think the module looks better in the documentation if its description
on the first line is "Library of toric varieties" without "a". Although I
can't really judge how bad it is from the English point of view not to
have an article ;-)
3) In general the module docstring should include more information
according to http://sagemath.org/doc/developer/conventions.html#headings-
of-sage-library-code-files
At the very least you should add yourself as the initial author, some
description of how it works also can be nice.
4) Unfortunately, it is not very consistent in Sage, but I try to follow
http://www.python.org/dev/peps/pep-0257/ cited in the beginning of
http://sagemath.org/doc/developer/conventions.html#python-coding-
conventions in the part of having a single line "command type" description
in the beginning of docstrings, i.e. in most cases "Return ..." In many
cases Sage uses "Returns ..." and the example in Developer's Guide is even
worse: "This function returns ..." I definitely do not require you to
follow the style I have chosen, but it can be nice if you follow it anyway
for consistency purposes ;-)
5) Following Developer's Guide, the input block of `Cube_deformation`
should look like
{{{
INPUT:
- ``k`` -- integer. The case ``k=0`` is :meth:`Cube_face_fan`.
}}}
6) I think it will be good for cross-linking compiled documentation if all
functions have one of these output blocks (in addition to the more
mathematical description of the output which they already have):
{{{
OUTPUT:
- :class:`toric variety
<sage.schemes.generic.toric_variety.ToricVariety_field>`.
OUTPUT:
- :class:`CPR-Fano toric variety
<sage.schemes.generic.fano_toric_variety.CPRFanoToricVariety_field>`.
}}}
7) I think it will be good if all examples displayed rays and names of
coordinates, like
{{{
sage: Conifold = toric_varieties.Conifold()
sage: Conifold
3-d affine toric variety
sage: Conifold.fan().ray_matrix()
[0 0 1 1]
[0 1 0 1]
[1 1 1 1]
sage: Conifold.gens()
(u, x, y, v)
}}}
It will make the compiled documentation more informative.
8) `check=self._check` should be passed to constructors of toric varieties
as well. Once we are confident that the rest of the code is correct and
set `self._check=False` by default, my hope is that there will be no "#
long time" comments in this module.
9) `sage -coverage` shows doctests of private functions as possibly wrong,
there should be a comment "# indirect doctest" in their examples.
10) It also shows `TestSuite` error which I propose to ignore so far.
There isn't really anything to test for such a class and I don't think
there is any point in pickling it. If somebody disagrees, at least it will
be easy for them to track down this module and do whatever is necessary.
Let me know what you think!
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/9245#comment:17>
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.
