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: |
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)
Change History (8)
comment:1 Changed 11 years ago by
Status: | new → needs_review |
---|---|
Type: | PLEASE CHANGE → defect |
comment:2 Changed 11 years ago by
comment:3 follow-up: 4 Changed 11 years ago by
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
Attachment: | trac_12875-category-fix_abvar_homspace-nt.patch added |
---|
comment:4 Changed 11 years ago by
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
Reviewers: | → Simon King |
---|---|
Status: | needs_review → positive_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:7 Changed 11 years ago by
Merged in: | → sage-5.1.beta0 |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
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).