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)

trac_12946.patch (1.2 KB) - added by mhansen 7 years ago.
trac_12946-review.patch (1.2 KB) - added by mhansen 7 years ago.
Removing some garbage from the top of the file

Download all attachments as: .zip

Change History (12)

comment:1 Changed 7 years ago by mhansen

  • Authors changed from Chris Berg to Mike Hansen
  • Component changed from PLEASE CHANGE to combinatorics
  • Status changed from new to needs_review

comment:2 Changed 7 years ago by andrew.mathas

  • Reviewers set to Andrew Mathas

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:

sage: Compositions(4, inner=Composition([3,1,2])).list()
Traceback (most recent call last):
...
TypeError: 'Composition_class' object is not callable


Of course, the fix is trivial: as you did with outer you need to replace line 984 with:

inner = list(kwargs['inner'])

Cheers, Andrew

Last edited 7 years ago by andrew.mathas (previous) (diff)

comment:3 Changed 7 years ago by andrew.mathas

  • Status changed from needs_review to needs_work

comment:4 Changed 7 years ago by andrew.mathas

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

Last edited 7 years ago by andrew.mathas (previous) (diff)

comment:5 Changed 7 years ago by andrew.mathas

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

Last edited 7 years ago by andrew.mathas (previous) (diff)

comment:6 Changed 7 years ago by mhansen

  • Status changed from needs_work to positive_review

Looks good to me.

comment:7 Changed 7 years ago by jdemeyer

  • Milestone changed from sage-5.3 to sage-5.4

comment:8 Changed 7 years ago by jdemeyer

Please add a more proper commit message ("Update of #12946" is quite meaningless).

Changed 7 years ago by mhansen

Changed 7 years ago by mhansen

Removing some garbage from the top of the file

comment:9 Changed 7 years ago by mhansen

I've fixed the commit message in trac_12946-review.patch.

comment:10 Changed 7 years ago by jdemeyer

  • Merged in set to sage-5.4.beta0
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.