#10529: dimension() and is_smooth() for algebraic subschemes of toric varieties
----------------------------------+-----------------------------------------
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 | Merged:
Work_issues: |
----------------------------------+-----------------------------------------
Changes (by novoselt):
* status: needs_review => needs_work
* reviewer: => Andrey Novoseltsev
Comment:
Hi Volker, I guess I better start reviewing this ticket. Little picks at
the first patch (trac_10529_toric_variety_library_names.patch).
1. It seems that instead of
{{{
- ``names`` -- an alias of ``coordinate_names`` for internal
use. Takes precedence if set.
}}}
you mean to say
{{{
- ``names`` -- an alias of ``coordinate_names`` for internal
use, cannot be specified together with ``coordinate_names``;
}}}
and I think that instead of
{{{
if not names is None:
assert coordinate_names is None, \
'You must not specify both coordinate_names and names!'
coordinate_names = names
}}}
it would be better to have
{{{
if names is not None:
if coordinate_names is not None:
raise ValueError('You must not specify both coordinate_names
and names!')
coordinate_names = names
}}}
2. When you write
{{{
dict_key = '_ '+name+' '+'_'.join(coordinate_names)
}}}
is it intentional that there is a space after the first underscore? Also,
did you mean to join coordinate names with underscores? Look at this:
{{{
sage: toric_varieties.__dict__
{}
sage: toric_varieties.P2()
2-d CPR-Fano toric variety covered by 3 affine patches
sage: toric_varieties.__dict__
{'_ P2 x_ _y_ _z': 2-d CPR-Fano toric variety covered by 3 affine patches}
}}}
If you meant to create a key `_P2_x_y_z`, I suggest using
`normalize_names` from `toric_variety` on coordinate names before joining
them and, of course, removing the space. Since you directly work with
`__dict__` it is fine even with spaces, but you cannot call this name as
an attribute (which can be sometimes convenient for debugging). It still
shows in TAB completion, however!
3. Wouldn't it be better to write
{{{
def dP6(self, names='x u y v z w'):
...
}}}
instead of
{{{
def dP6(self, names=None):
if names is None:
names = 'x u y v z w'
...
}}}
4. This new argument `names` must be documented in `dP6` etc.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/10529#comment:5>
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.
