Opened 6 years ago
Closed 6 years ago
#16289 closed defect (fixed)
Fix ZZ.cartesian_product(...)
Reported by: | nthiery | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-6.3 |
Component: | combinatorics | Keywords: | |
Cc: | sage-combinat, ncohen, vdelecroix | Merged in: | |
Authors: | Nicolas M. Thiéry | Reviewers: | Nathann Cohen |
Report Upstream: | N/A | Work issues: | |
Branch: | c9a7e67 (Commits) | Commit: | c9a7e6728d1689b828edb5b367d69a89a9a0cf66 |
Dependencies: | #16280, #16288, #16269 | Stopgaps: |
Description
This fails due to an inadvertent change introduced by #12959:
sage: ZZ.cartesian_product(GF(2), NN) Traceback (most recent call last) ... AttributeError: type object 'sage.rings.integer_ring.IntegerRing_class' has no attribute 'CartesianProduct'
This ticket reverts this change and adds a doctest.
Change History (15)
comment:1 Changed 6 years ago by
- Branch set to u/nthiery/16269-optimized
comment:2 Changed 6 years ago by
- Branch changed from u/nthiery/16269-optimized to u/nthiery/16289/fix_zz_cartesian_product
comment:3 Changed 6 years ago by
- Commit set to c9a7e6728d1689b828edb5b367d69a89a9a0cf66
- Status changed from new to needs_review
comment:4 Changed 6 years ago by
- Dependencies set to #16280
comment:5 Changed 6 years ago by
This branch apparently includes #16269 as a dependency... But perhaps that is not a problem anyway as it is already reviewed ?
Nathann
comment:6 Changed 6 years ago by
comment:7 follow-up: ↓ 8 Changed 6 years ago by
- Dependencies changed from #16280 to #16280, #16288, #16269
(I set the dependencies that appear in the branch, please update it if you rewrite the branch as some of these dependencies are not necessary. But of course possibly we don't care for as long as everything gets reviewed)
comment:8 in reply to: ↑ 7 Changed 6 years ago by
Replying to ncohen:
(I set the dependencies that appear in the branch, please update it if you rewrite the branch as some of these dependencies are not necessary. But of course possibly we don't care for as long as everything gets reviewed)
There actually is a small semantic dependency upon #16269 since it changes the output of one of the doctests of this ticket. For #16288, oh well. Let's just keep the branch as it is. As you say we don't really care since everything is positive reviewed or close enough to be so.
comment:9 follow-up: ↓ 10 Changed 6 years ago by
Yoooooo !
Looks totally nice... Do you have any idea why there was a __class__
there ? It does not make any sense to me O_o
Nathann
comment:10 in reply to: ↑ 9 Changed 6 years ago by
Replying to ncohen:
Looks totally nice... Do you have any idea why there was a
__class__
there ? It does not make any sense to meO_o
Go figure ... This looks a bit more like a call to a class method rather than a call to a usual method, since we pass explicitly self
as first argument. So maybe the point was to emphasize that? Anyway we don't need it, so it's as good without it.
comment:11 Changed 6 years ago by
Yep it seems clearer this way.... O_O
Okay, good to go... I need to calm down a bit. I just "reviewed" #16246.
Did you check the doctests ?
Nathann
comment:12 Changed 6 years ago by
Yup, all long tests passed for me!
comment:13 Changed 6 years ago by
- Reviewers set to Nathann Cohen
- Status changed from needs_review to positive_review
Okay, so positive review and I will check the docstest again on my office's computer again just in case, now that Leandro turned it on again.
Nathann
comment:14 Changed 6 years ago by
- Milestone changed from sage-6.2 to sage-6.3
comment:15 Changed 6 years ago by
- Branch changed from u/nthiery/16289/fix_zz_cartesian_product to c9a7e6728d1689b828edb5b367d69a89a9a0cf66
- Resolution set to fixed
- Status changed from positive_review to closed
Last 10 new commits:
trac #16269: Working around "officially not a bug"
trac #16269: missing __iter__
trac #16269: Merged with 6.2.rc1
trac #16269: Reviewer's remarks
#16280: Fix call for FiniteEnumeratedSet's of plain Python objects
#16280: Trivial doctest fixes
trac #16269: Merged with #16280
Trac 16269: little improvements + line folding in the doctest output
Trac 16269 (or follow up): optimize CartesianProduct._cartesian_product_of_elements
#16289: Fix ZZ.cartesian_product(...)