#11599: Wrap fan morphism in toric morphism
----------------------------------+-----------------------------------------
Reporter: vbraun | Owner: AlexGhitza
Type: enhancement | Status: needs_work
Priority: major | Milestone: sage-5.0
Component: algebraic geometry | Keywords:
Work_issues: | Upstream: N/A
Reviewer: Andrey Novoseltsev | Author: Volker Braun
Merged: | Dependencies:
----------------------------------+-----------------------------------------
Changes (by novoselt):
* status: needs_review => needs_work
Comment:
For the last patch:
1. In `schemes.rst` extra trailing spaces are added to one line (which I
think was recently prohibited ;-)) and `toric_morphism` is added twice -
probably it should be only once?
2. In `scheme.py` it adds `cached_method`, but it was already imported,
so this hunk should not be there.
3. In `toric_homset.py`: I think that generic description should not be
duplicated in the module docstring. On the other hand AUTHOURS, EXAMPLES
and license sections should be added.
4. `__init__` in the beginning does not have a docstring at all. It would
be nice also to add some clarifying comment what exactly is achieved by
`register_conversion` there (although maybe I am supposed to know
that?...)
5. In the example of `_element_constructor_` can we please not use `Hom`
as the name for `dP8.Hom(P2)`? `Hom` is a top-level function and
overwriting it is a bit confusing, especially since the example is quite
long. How about just `H` instead?
6. In `_repr_defn` of toric morphism lines like "Defined by sending the
Rational polyhedral fan in 2-d lattice N to Rational polyhedral fan in 1-d
lattice N." are constructed. "the" in front of domain but not codomain is
a bit strange and I think we should remove it since fans are capitalized.
Or put it in both places, but it makes strings longer ;-)
7. In `_homset_class` of `toric_variety.py` the old doctest is deleted
and it is actually no longer working. Without direct calls to private
functions, `H([z0, z1, z0, z3])` is also not working, which bugs me quite
a bit. The traceback goes to `NotImplementedError` and includes
`Set_generic.__call__` which I have complained a bit in
http://trac.sagemath.org/sage_trac/ticket/11599#comment:30.
These are all the comments that I have, will upload new reviewer patch
shortly.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/11599#comment:36>
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.
