#9054: create a class for basic function_field arithmetic for Sage
---------------------------------------------------------------------------------+
Reporter: was
| Owner: was
Type: enhancement
| Status: new
Priority: major
| Milestone: sage-wishlist
Component: algebra
| Resolution:
Keywords:
| Work_issues:
Upstream: N/A
| Reviewer:
Author: William Stein, Robert Bradshaw, Maarten Derickx, Moritz Minzlaff
| Merged:
Dependencies:
|
---------------------------------------------------------------------------------+
Comment(by SimonKing):
Here are some comments on [attachment:trac_9054-all-parts.patch]:
* Please remove the `__contains__` method from the category
`FunctionFields`. Containment in categories should rely on the default
implementation, unless there is a compelling reason to do otherwise.
Even worse, your containment test is ultimately based on testing class
inheritance (namely in the function `is_FunctionField`). That totally
undermines the category framework. It must be possible for an object to be
a function field even without inheriting from
`sage.rings.function_field.function_field.FunctionField`.
The default implementation of `F in FunctionFields()` relies on the
category of F: The containment test returns True if and only if
`F.category()` is a sub-category of `FunctionFields()`. That should be
much better, from a mathematical point of view, than testing class
inheritance!
* You should add a test of the form `TestSuite(F).run()`, where F is a
function field. The test suite is formed by some generic tests defined in
the category framework and includes many sanity tests (such as pickling
for the field and its elements, associativity, commtativity, ...). If you
can think of specific tests for function fields, then you should add
methods named `_test_...` as parent or element methods of
`sage.categories.function_fields.FunctionFields`. Such methods will be
automatically called when running `TestSuite(F).run()`.
* You should also add a test of the form `loads(dumps(F)) is F`, in order
to test uniqueness of parent structures; if I recall correctly, the test
suite from the category would only test `loads(dumps(F))==F`.
* It should not be needed to have a function `is_FunctionField` (that
just tests class inheritance) - `F in FunctionFields()` is a better test,
IMHO. If you do want to preserve `is_FunctionField` then please do not
simply put it in the global name space. At least, it should be deprecated,
similar to `is_Ring` being deprecated. There is a function decorator to do
so.
* In the doc test for the `_element_constructor_` method, you explicitly
call the method. I think it should better be an indirect test (after all,
the documentation is supposed to show how the user is supposed to work
with stuff). Hence, not
`L._element_constructor_(L.polynomial_ring().gen())` but
`L(L.polynomial_ring().gen()) #indirect doctest`.
* I already mentioned, since `FunctionField` is derived from
`sage.rings.ring.Field`, that `Field.__init__(...)` should be called. It
could be that this only works when #9138 is used. Just calling
`ParentWithGens.__init__` may be insufficient.
* There are several methods, such as polynomial_ring or vector_space,
that use a hand-made cache. Please use the @cached_method decorator
instead! That has several reasons.
1. It is more easy. You don't need to manually update attributes.
2. With #11115, the @cached_method decorator is rewritten in Cython and
provides a faster cache than anything you could possibly create with
Python.
* Is there a reason why you have a method `base_field` that simply
returns the function field itself? From the behaviour of the `base_ring`
method of polynomial rings, I would rather expect that
`FunctionField(QQ,['t']).base_field()` returns the rational field.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/9054#comment:37>
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.
