#6863: Implement cusps over number fields
-----------------------------+----------------------------------------------
Reporter: cremona | Owner: craigcitro
Type: enhancement | Status: needs_work
Priority: major | Milestone: sage-4.3.1
Component: modular forms | Keywords: number field modular
Work_issues: | Author:
Upstream: N/A | Reviewer:
Merged: |
-----------------------------+----------------------------------------------
Changes (by craigcitro):
* status: needs_review => needs_work
Comment:
So this code looks '''really''' nice -- I'm really happy to see this
functionality in Sage, and I'm very impressed with the completeness of the
functionality, and the documentation/doctests.
However, I think this code does need some purely cosmetic changes before
we merge it. Here are my first thoughts:
* I think that we shouldn't provide a separate "NFCusp" function to the
user -- they should just call Cusp, which could deduce the base ring from
the arguments, and take an optional `base_ring=` parameter.
* For that matter, we should probably just have a `Cusp_rationals` and
`Cusp_number_field` class which both inherit from a `Cusp_generic`, and
unify as much of this code as possible with the existing cusps code.
* Similar comments to the previous two also apply to `NFCusps`, i.e. the
parent for cusps.
* I really like that there are convenient functions for getting a
corresponding number field element from a cusp -- however, we should
probably do the extra work to make the coercion framework know about
these, so that you could simply do something like `K(c)` to get a
representative for the cusp.
* In fact, I might go one step further and say that we shouldn't really
need to add any new functions into the global namespace -- i.e.
`sage/modular/all.py` shouldn't need to import anything directly, except
maybe the `_clear_cache` functions. All of the things that do get imported
are analogues for number fields of existing bits, which should be able to
be handled transparently. As far as the `_clear_cache` stuff, maybe we
could just have those be available as methods on the parents? I wouldn't
mind `_Cusp_clear_cache_` or something at top-level, as long as it was
prefixed with `_`.
I think that's it at a first go, but it's probably enough to get started
moving in the right direction. John and/or Maite, do you want to make
these changes, or would you rather I do it? I won't get to it for about
two weeks, but I'm happy to do it if you won't have time before then.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/6863#comment:3>
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.
