#5891 closed enhancement (fixed)
Categories for the working mathematics programmer — at Version 29
Reported by: | nthiery | Owned by: | nthiery |
---|---|---|---|
Priority: | major | Milestone: | sage-4.3 |
Component: | categories | Keywords: | categories parents |
Cc: | sage-combinat, roed, saliola | Merged in: | sage-4.3.alpha0 |
Authors: | Nicolas M. Thiéry, Teresa Gomez-Diaz | Reviewers: | Robert Bradshaw, Craig Citro, Florent Hivert, David Kohel, David Roe, Anne Schilling, William Stein, Javier Vengoroso |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
This (series of) patch(es) extends the Sage category framework as a design pattern for organizing generic code.
Latest version of the patches:
- categories-framework-nt.patch (reviewer: David Roe; review essentially complete up to some doctests)
- categories-categories-nt.patch (reviewer: Florent Hivert, Javier, David Kohel, Anne Schilling, ...) See CategoriesCategoriesReview
- categories-fixsagelib-nt.patch (positive review: Robert Bradshaw) updates to the sage library (import fixes, ...)
- categories-fix-combinat-nt.patch (positive review: Florent)
- categories-numberfield_homset-nt.patch (reviewer: William)
- categories-unpickle_backward_compatibility_aliases-nt.patch (positive review: Florent)
Status and roadmap: http://trac.sagemath.org/sage_trac/wiki/CategoriesRoadMap
Change History (35)
comment:1 Changed 11 years ago by
- Description modified (diff)
comment:2 Changed 11 years ago by
- Cc roed added
comment:3 Changed 11 years ago by
- Cc saliola added
comment:4 Changed 11 years ago by
- Description modified (diff)
comment:5 Changed 11 years ago by
- Description modified (diff)
comment:6 Changed 11 years ago by
- Description modified (diff)
comment:7 Changed 11 years ago by
- Description modified (diff)
comment:8 follow-up: ↓ 11 Changed 11 years ago by
comment:9 Changed 11 years ago by
- Description modified (diff)
comment:10 Changed 11 years ago by
- Description modified (diff)
comment:11 in reply to: ↑ 8 Changed 11 years ago by
Replying to robertwb:
Where is categories-nt.patch itself?
On the sage-combinat patch server. It changes too often to keep it updated on trac. I highlighted the link on top of the description (and improved the ReSTing)
comment:12 Changed 11 years ago by
- Description modified (diff)
comment:13 Changed 11 years ago by
- Description modified (diff)
comment:14 Changed 11 years ago by
- Status changed from new to assigned
Changed 11 years ago by
Changed 11 years ago by
Changed 11 years ago by
Changed 11 years ago by
Changed 11 years ago by
Changed 11 years ago by
comment:15 Changed 11 years ago by
There are the patches. They are file-orthogonal, so they should apply in any order.
comment:16 Changed 11 years ago by
Note: to get the latest version, please use the patch server.
comment:17 follow-up: ↓ 19 Changed 11 years ago by
- Status changed from new to needs_work
NOTES:
(1) Post the latest version here -- I don't want to mess with the patch server.
(2) It says "Experts: please redefine this properly and/or put CC/RR/... in NumberFields?()". I number field is by definition a finite extension of QQ, but CC and RR are infinite extensions of QQ. So we can't put them in that category. Having a function is_NumberFieldHomsetCodomain does seem like a good workaround for now.
(3) The function is_NumberFieldHomsetCodomain in the patch posted here doesn't have any documentation or doctests. Please add them.
(4) I would change these two lines:
143 if is_Field(codomain): 144 return True
to the single line
143 return is_Field(codomain)
which should be functionally the same, and clearer to read.
(5) in a similar spirit, I would change
145 except: 146 pass 147 return False
to just
145 except: 146 return False
which is again clearer and equivalent.
(6) I'm puzzled by this in your number_field_rel.py patch:
543 return NotImplemented
What is NotImplemented?? It's not defined in the number_field_rel.py file in sage-4.2.
comment:18 Changed 11 years ago by
- Description modified (diff)
comment:19 in reply to: ↑ 17 ; follow-up: ↓ 24 Changed 11 years ago by
- Description modified (diff)
- Reviewers set to Robert Bradshaw, Florent Hivert, David Kohel, David Roe, Anne Schilling, Javier Vengoroso
- Status changed from needs_work to needs_review
Replying to was:
NOTES:
(1) Post the latest version here -- I don't want to mess with the patch server.
I just added direct links in the description. I will post the patches shortly when they will be final.
(2) It says "Experts: please redefine this properly and/or put CC/RR/... in NumberFields?()". I number field is by definition a finite extension of QQ, but CC and RR are infinite extensions of QQ. So we can't put them in that category. Having a function is_NumberFieldHomsetCodomain does seem like a good workaround for now.
Ok.
(3) The function is_NumberFieldHomsetCodomain in the patch posted here doesn't have any documentation or doctests. Please add them.
Oops. Will do.
(4) (5) (6)
Yes better. Will do.
(6) I'm puzzled by this in your number_field_rel.py patch:
543 return NotImplementedWhat is NotImplemented?? It's not defined in the number_field_rel.py file in sage-4.2.
It's a builtin python object. Anyway, the function now raises a TypeError?, per the latest _Hom_ protocol.
comment:20 Changed 11 years ago by
- Component changed from misc to algebra
- Status changed from needs_review to needs_work
comment:21 Changed 11 years ago by
- Description modified (diff)
- Reviewers changed from Robert Bradshaw, Florent Hivert, David Kohel, David Roe, Anne Schilling, Javier Vengoroso to Robert Bradshaw, Craig Citro, Florent Hivert, David Kohel, David Roe, Anne Schilling, William Stein, Javier Vengoroso
- Status changed from needs_work to needs_review
comment:22 Changed 11 years ago by
- Description modified (diff)
comment:23 Changed 11 years ago by
- Description modified (diff)
comment:24 in reply to: ↑ 19 Changed 11 years ago by
comment:25 Changed 11 years ago by
- Component changed from algebra to categories
comment:26 Changed 11 years ago by
- Merged in set to sage-4.3.alpha0
- Resolution set to fixed
- Status changed from needs_review to closed
Merged the patches from changeset e70487186111. They'll be posted on here in a bit.
comment:27 Changed 11 years ago by
- Milestone changed from sage-combinat to sage-4.3
comment:28 Changed 10 years ago by
- Report Upstream set to N/A
comment:29 Changed 9 years ago by
- Description modified (diff)
Where is categories-nt.patch itself?