Opened 7 years ago
Closed 7 years ago
#12875 closed defect (fixed)
Fix the homset category initialization for ModularAbelianVariety's homspaces
| Reported by: | nthiery | Owned by: | craigcitro |
|---|---|---|---|
| Priority: | major | Milestone: | sage-5.1 |
| Component: | modular forms | Keywords: | categories, abelian varieties |
| Cc: | sage-combinat | 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 7 years ago by
- Status changed from new to needs_review
- Type changed from PLEASE CHANGE to defect
comment:2 Changed 7 years ago by
comment:3 follow-up: ↓ 4 Changed 7 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 7 years ago by
comment:4 in reply to: ↑ 3 Changed 7 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 7 years ago by
- Reviewers set to Simon King
- Status changed from needs_review to 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:6 Changed 7 years ago by
Thanks!
comment:7 Changed 7 years ago by
- Merged in set to sage-5.1.beta0
- Resolution set to fixed
- Status changed from positive_review to 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).