Opened 7 years ago
Closed 7 years ago
#12946 closed defect (fixed)
Bug in Compositions
Reported by: | chrisjamesberg | Owned by: | tbd |
---|---|---|---|
Priority: | minor | Milestone: | sage-5.4 |
Component: | combinatorics | Keywords: | Compositions |
Cc: | Merged in: | sage-5.4.beta0 | |
Authors: | Mike Hansen | Reviewers: | Andrew Mathas |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
When using the optional 'outer' argument, Compositions gives a bug if 'outer' is itself a composition (as opposed to a list). Also needs to be fixed for 'inner'.
For instance:
sage: c = Compositions(3, outer = Composition([3,2]))
sage: for x in c: ....: print x ....:
TypeError? Traceback (most recent call last)
/Applications/sage/devel/sage-combinat/.hg/patches/<ipython console> in <module>()
/Applications/sage/local/lib/python2.7/site-packages/sage/combinat/integer_list.pyc in iter(self)
1023 args = self.build_args() 1024 for n in self.n_range:
-> 1025 l = first(n, *args)
1026 while l is not None: 1027 yield self._element_constructor_(l)
/Applications/sage/local/lib/python2.7/site-packages/sage/combinat/integer_list.pyc in first(n, min_length, max_length, floor, ceiling, min_slope, max_slope)
72 return None 73
---> 74 if ceiling(min_length) == 0 and max_slope <= 0:
75 return None 76
/Applications/sage/local/lib/python2.7/site-packages/sage/combinat/integer_list.pyc in <lambda>(i)
994 """ 995 return [self.min_length, self.max_length,
--> 996 lambda i: self.floor(i-1), lambda i: self.ceiling(i-1),
997 self.min_slope, self.max_slope] 998
TypeError?: 'Composition_class' object is not callable
Attachments (2)
Change History (12)
comment:1 Changed 7 years ago by
- Component changed from PLEASE CHANGE to combinatorics
- Status changed from new to needs_review
comment:2 Changed 7 years ago by
- Reviewers set to Andrew Mathas
comment:3 Changed 7 years ago by
- Status changed from needs_review to needs_work
comment:4 Changed 7 years ago by
Hi Mike,
To save time I just uploaded a reviewer patch which fixes the inner problem above and adds a test for it. If you are happy with this, and if the tests all pass, then I'm happy to give it a positive review.
Andrew
--
For the patchbot:
Apply trac_12946.patch trac_12946-review.patch
comment:5 Changed 7 years ago by
All of the tests passed so if you are happy with the review patch I'll give this a positive review.
Andrew
Apply trac_12946.patch trac_12946-review.patch
comment:6 Changed 7 years ago by
- Status changed from needs_work to positive_review
Looks good to me.
comment:7 Changed 7 years ago by
- Milestone changed from sage-5.3 to sage-5.4
comment:8 Changed 7 years ago by
Please add a more proper commit message ("Update of #12946" is quite meaningless).
Changed 7 years ago by
comment:9 Changed 7 years ago by
I've fixed the commit message in trac_12946-review.patch.
comment:10 Changed 7 years ago by
- Merged in set to sage-5.4.beta0
- Resolution set to fixed
- Status changed from positive_review to closed
The documentation on what outer and inner do is not particularly clear to me, so so I think that it would not hurt to explain this better, but I'll leave that decision up to you.
More importantly, the patch doesn't work when inner is a composition, so I am marking the patch as
needs work
:Of course, the fix is trivial: as you did with outer you need to replace line 984 with:
Cheers, Andrew