Opened 11 years ago

Closed 11 years ago

#12875 closed defect (fixed)

Fix the homset category initialization for ModularAbelianVariety's homspaces

Reported by: Nicolas M. Thiéry Owned by: Craig Citro
Priority: major Milestone: sage-5.1
Component: modular forms Keywords: categories, abelian varieties
Cc: Sage Combinat CC user Merged in: sage-5.1.beta0
Authors: Nicolas M. Thiéry Reviewers: Simon King
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

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.

Attachments (1)

trac_12875-category-fix_abvar_homspace-nt.patch (4.7 KB) - added by Nicolas M. Thiéry 11 years ago.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 11 years ago by Nicolas M. Thiéry

Status: newneeds_review
Type: PLEASE CHANGEdefect

comment:2 Changed 11 years ago by Nicolas M. Thiéry

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).

comment:3 Changed 11 years ago by Simon King

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.

Changed 11 years ago by Nicolas M. Thiéry

comment:4 in reply to:  3 Changed 11 years ago by Nicolas M. Thiéry

Replying to SimonKing:

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!

comment:5 Changed 11 years ago by Simon King

Reviewers: Simon King
Status: needs_reviewpositive_review

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.

comment:6 Changed 11 years ago by Nicolas M. Thiéry

Thanks!

comment:7 Changed 11 years ago by Jeroen Demeyer

Merged in: sage-5.1.beta0
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.