I don't think this is the correct fix as the input is valid. It should either:

1. Be the empty set (so the __iter__ simply returns).
2. Iterate starting from the lex min less than the given partition.

I am leaning towards (1) as it is easier to implement and most people only intend to compare partitions of the same size. However, (2) fits with what the doc says and lex order makes sense for all partitions (which is useful to do sometimes).

1. Be the empty set (so the __iter__ simply returns).
2. Iterate starting from the lex min less than the given partition.

I am leaning towards (1) as it is easier to implement and most people only intend to compare partitions of the same size. However, (2) fits with what the doc says and lex order makes sense for all partitions (which is useful to do sometimes).

It looks like (2) is in keeping with what the ending kwarg does.

sage: Partitions(4, ending=[2,1])
Partitions of the integer 4 ending with [2, 1]
sage: list(_)
[[4], [3, 1], [2, 2], [2, 1, 1]]

For consistency's sake, I'll do (2).

Another bug with the current approach:

sage: P = Partitions(5, starting=[3,1]); print(P); list(P)
Partitions of the integer 5 starting with [3, 1]
[[3, 1], [2, 2], [2, 1, 1], [1, 1, 1, 1]]
sage: Partition([3,1]) in P
False

Indeed, it seems like we should go with (2); good find.

Indeed, it seems like we should go with (2); good find.

It wasn't too bad to fix - I just had to change the Partitions_starting.first method.

Actually I think there is still a logical mistake I'm making. It only makes sense to have starting be too big. So if starting is too small I should implement option (1), and if it is too big option (2).

For example, [2,1,1] is not less than [2,1] because 2 <= 2, 1<= 1, 1 >= 0. But the last comment I pushed wouldn't catch that (I went back to add a test and it failed in _contains_ for exactly this reason).

Right. I believe you can just add the trailing 1s, then return the next partition in lex order as the starting partition.

Right. I believe you can just add the trailing 1s, then return the next partition in lex order as the starting partition.

Ah right that makes sense. I think its good now. I did also notice the following issue:

sage: P = Partitions(4, ending = [5])
sage: P.list()
[[4]]
sage: Partition([4]) in P
False

I think this should be an empty list. I think I can fix it by adding an __iter__ method to Partitions_ending (mirroring the fix I did for Partitions_starting). Should I do that on this issue/PR, or another one?

You shouldn't need the __iter__ method at all. Python(?) automagically uses first() and next() to build an iterator. I would just change the next() to be part <= self._ending and have a check in first of something like

if self._ending and self.n <= self._ending[0] and not (self.n == self._ending[0] and len(self._ending) == 1):
    return None

Although it is up to you if you want do it here or on a separate PR.

You shouldn't need the __iter__ method at all. Python(?) automagically uses first() and next() to build an iterator. I would just change the next() to be part <= self._ending and have a check in first of something like

if self._ending and self.n <= self._ending[0] and not (self.n == self._ending[0] and len(self._ending) == 1):
    return None

Although it is up to you if you want do it here or on a separate PR.

When I try to do that, I get the following test failing:

Failed example:
    Partitions(4, ending=[5]).list()
Expected:
    []
Got:
    [None]

I had the same issue with the Partitions_starting and adding __iter__ was able to fix it. Is that the wrong fix for Partitions_starting too?

Ah, we just need to make a small additional modification to EnumeratesSets.ParentMethods._iterator_from_next()

f = self.first()
while not (f is None or f is False):
    yield f
    try:
        f = self.next(f)
    except (TypeError, ValueError):
        f = None

I also cleaned up the implementation a bit. Now if first() returns None, then it will create an empty list.

Ah, we just need to make a small additional modification to EnumeratesSets.ParentMethods._iterator_from_next()

f = self.first()
while not (f is None or f is False):
    yield f
    try:
        f = self.next(f)
    except (TypeError, ValueError):
        f = None

I also cleaned up the implementation a bit. Now if first() returns None, then it will create an empty list.

Ah awesome thanks. Changes are made and ready for review.

Documentation preview for this PR is ready! 🎉
Built with commit: 4ec3ad4