#9055: Moving/cleaning enumeration functions for points on schemes
----------------------------------+-----------------------------------------
Reporter: cturner | Owner: AlexGhitza
Type: enhancement | Status: needs_work
Priority: minor | Milestone:
Component: algebraic geometry | Keywords: rational points enumeration
Author: | Upstream: N/A
Reviewer: | Merged:
Work_issues: |
----------------------------------+-----------------------------------------
Changes (by novoselt):
* status: needs_review => needs_work
Comment:
I just glanced at the patch, but I have the following comments:
1) "Set of homomorphisms between two schemes" should stay in the original
file, it is the docstring of the module describing what does it do.
2) There should be a new description (hopefully, more informative) in the
new module. The first line is how this module will be listed in the
documentation, the rest can be any text. Also, there should be the
copyright/license notice and AUTHORS field in the module docline (stating
who has written these functions initially and that you have improved and
documented them).
3) There are some doctests with very long output strings. This leads to
not very good looking formatting of the documentation. You can insert line
breaks in the output and doctests still will run fine. I would suggest
doing it to keep line length under 80 (or even 60, excluding leading
spaces,) characters.
4) Did you try to compile a documentation with this module? I didn't, but
it seems to me that there are quite a few problems with indentations and
"::"s. Also, now that you have actually added the docstrings to these
functions, it would make sense to include this module into the auto-
generated documentation tree.
5) Is there any specific reason for importing these functions in the
homset module right before their use? It is my strong belief that all
imports should be in the beginning of the file to avoid repetitions and to
make it clear what is used by this module.
I think that 1), 2), and 4) are mandatory, while 3) and 5) may reflect my
personal taste (although I tried to explain why do I think that they make
sense).
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/9055#comment:4>
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.
