#8678: Improvements for morphisms of ModulesWithBasis
-------------------------------------+-------------------------------------
Reporter: nthiery | Owner: nthiery
Type: defect | Status: needs_info
Priority: major | Milestone: sage-6.4
Component: categories | Resolution:
Keywords: homsets, module | Merged in:
morphisms, days64 | Reviewers: Franco Saliola
Authors: Nicolas M. Thiéry | Work issues:
Report Upstream: N/A | Commit:
Branch: | a2f158c1ae29a9ff70e295e1dc028d1718fd09ac
u/nthiery/categories/module- | Stopgaps:
morphisms-8678 |
Dependencies: #10668, #17160 |
-------------------------------------+-------------------------------------
Comment (by nthiery):
Hi Franco!
Replying to [comment:60 saliola]:
> Here is my review. I pushed some typo fixes, improved some of the
> documentation and added a few doctests.
Thanks! I double checked your changes and am happy with them. This
prompted a second pass of little things here and there to make the doc
more uniform. I also implemented the two we had discussed.
> Here are a few issues and questions:
>
> - I don't understand the comment in
``src/sage/categories/finitely_generated_semigroups.py``:
> {{{
> # TODO: update transitive ideal
> }}}
The code uses TransitiveIdeal which is being deprecated in favor of
RecursivelyEnumeratedSet. I have added this as comment on #17160.
> - a couple of doctests include the following line:
> {{{
> sage: import __main__; __main__.f = f
> }}}
> can you explain why this is necessary?
A function defined interactively is not picklable, which prevents us
from using it to test the pickling of objects built upon them. This
classical trick fakes f being defined in a Python module.
> - line 1015 of ``src/sage/modules/module_with_basis_morphism.py``: the
doc says this should work over an ring, so perhaps the following is not a
valid assumption?
> {{{
> c = c / s[j] # the base ring is a field
> }}}
The documentation mentions:
{{{
- ``self`` -- a triangular morphism over a field, or a
unitriangular morphism over a ring
}}}
which is tested a couple lines above:
{{{
if G.base_ring() not in Fields and not self._unitriangular:
raise NotImplementedError, "coreduce for a triangular but not
unitriangular morphism over a ring"
}}}
> - the doctests don't pass, but this seems to be related to #17160
Yup. Next step is to cleanup #17160. And then we will know better if
there are a couple trivial doctests that need to be updated here.
Cheers,
Nicolas
--
Ticket URL: <http://trac.sagemath.org/ticket/8678#comment:64>
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 unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/d/optout.
