Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the homset category initialization for ModularAbelianVariety's homspaces #12875

Closed
nthiery opened this issue Apr 24, 2012 · 8 comments
Closed

Comments

@nthiery
Copy link
Contributor

nthiery commented Apr 24, 2012

Before the patch, the following was wrong (probably introduced by #9138):

    sage: End(J0(37)).homset_category()
    Join of Category of hom sets in Category of sets and Category of rings

After the patch:

    sage: End(J0(37)).homset_category()
    Category of modular abelian varieties over Rational Field

In both cases, we have, as desired:

    sage: End(J0(37)).category()
    Join of Category of hom sets in Category of sets and Category of rings

By the way, this removes a direct call to _Hom_, using Hom instead, preparing for #11935.

Note: #11935 depends on this ticket.

CC: @sagetrac-sage-combinat

Component: modular forms

Keywords: abelian varieties

Author: Nicolas M. Thiéry

Reviewer: Simon King

Merged: sage-5.1.beta0

Issue created by migration from https://trac.sagemath.org/ticket/12875

@nthiery
Copy link
Contributor Author

nthiery commented Apr 25, 2012

comment:2

For the record: all tests passed on 5.0.beta13, with a couple unrelated sage-combinat patches just above (except for one doctest failure in sagedoc caused by those patches).

@simon-king-jena
Copy link
Member

comment:3

Some small criticism: The commit message does not mention the ticket number. Apart from that, the patch looks fine, and I am now running doctests.

@nthiery
Copy link
Contributor Author

nthiery commented Apr 26, 2012

@nthiery
Copy link
Contributor Author

nthiery commented Apr 26, 2012

comment:4

Replying to @simon-king-jena:

Some small criticism: The commit message does not mention the ticket number.

Fixed in the updated patch. Thanks for catching this, and for the quick review!

@simon-king-jena
Copy link
Member

Reviewer: Simon King

@simon-king-jena
Copy link
Member

comment:5

Thank you for updating the commit message! All tests pass, with sage-5.1.notebook, the patch applied after the patches from #12808 (I was to lazy to remove them). The patch looks fine, thus, I give it a positive review.

@nthiery
Copy link
Contributor Author

nthiery commented Apr 26, 2012

comment:6

Thanks!

@jdemeyer
Copy link

jdemeyer commented May 6, 2012

Merged: sage-5.1.beta0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants