k-Combinations with negative k

[MarcusAichmayr]

Steps To Reproduce

Call

sage: list(Combinations(3, -1))

Expected Behavior

sage: list(Combinations(3, -1))
[]

Actual Behavior

Instead, we obtain a ValueError:

sage: list(Combinations(3, -1))
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
/tmp/ipykernel_8533/142333014.py in <module>
----> 1 list(Combinations(Integer(3), -Integer(1)))

/usr/lib/python3/dist-packages/sage/combinat/combination.py in _iterator(self, items, n)
    451             [[1, 2, 3], [1, 2, 4], [1, 3, 4], [2, 3, 4]]
    452         """
--> 453         for combination in itertools.combinations(items, n):
    454             yield list(combination)
    455 

ValueError: r must be non-negative

Additional Information

How many k-combinations of {1, ..., n} exist for k = -1? According to Wikipedia, this number is equal to the binomial coefficient (n, k). For k = -1, we have (n, -1) = 0. Hence, there are no -1-combinations and we should obtain the empty list.

Environment

• OS: Ubuntu 22.04 (CoCalc)
• Sage Version: 10.4

Checklist

• I have searched the existing issues for a bug report that matches the one I want to file, without success.
• I have read the documentation and troubleshoot guide

[pawani27]

@MarcusAichmayr For this we can insert a condition to check for negative values and return an empty list if this condition is true. I will add a PR with the required changes.

[abhishekdubey369]

@MarcusAichmayr I would like to work on this assign me

[DaveWitteMorris]

A user who passes a negative number for r has probably made a mistake, so I think the ValueError is reasonable. (I think the error comes from itertools.combinations, and I think staying consistent with that has value.) If this is going to be changed, I think there will need to be a deprecation period first. But I personally am not at all convinced that it should be changed, and I think just documenting the behaviour might be best.

[abhishekdubey369]

Works can I document the same

…

On Fri, 31 Jan, 2025, 06:55 DaveWitteMorris, ***@***.***> wrote: A user who passes a negative number for r has probably made a mistake, so I think the ValueError is reasonable. (I think the error comes from itertools.combinations, and I think staying consistent with that has value.) If this is going to be changed, I think there will need to be a deprecation period first. But I personally am not at all convinced that it should be changed, and I think just documenting the behaviour might be best. — Reply to this email directly, view it on GitHub <#39411 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AOY55UMAFZQQXXXM2UBJH632NLGH5AVCNFSM6AAAAABWFCQV22VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMRWGA4DCMRXGU> . You are receiving this because you commented.Message ID: ***@***.***>

[pawani27]

@DaveWitteMorris shouldn't the error be also returned in the case of list(Combinations(-1,3))? This is just an example. Though the output I'm getting is an empty list.

[DaveWitteMorris]

Yes, I think an error would be better. However, this is a change in behaviour that is not fixing a clear bug, so there would need to be a deprecation. For the first year, the function needs to behave as it always did, but print a warning message that the default behaviour will change in the future.

This could be handled by adding a keyword argument allow_negative=None. If the user passes a negative value, then this argument needs to be checked:

• If allow_negative is None, then [] is returned and a brief deprecation warning is printed, telling them that the default value of allow_negative will be false in the future, but they can use allow_negative=True.
• If allow_negative is True, then [] is returned and no warning is printed.
• If allow_negative is False, then a ValueError is raised.

See Deprecation and sage.misc.superseded.deprecation for more information about deprecation.

[pawani27]

Okay I will work on this.

[DaveWitteMorris]

@pawani27: Didn't abhishekdubey369 reply before you did that they were going to work on this issue? Please coordinate with them so there isn't wasted effort.

[pawani27]

I actually did start working on this as you can see from my very first comment on this issue but no problem if @abhishekdubey369 has already made progress on this. @abhishekdubey369 Please let me know as to your preference.

[abhishekdubey369]

@pawani I have started working on it . If you want we can coordinate and work together on this. Thanks

…

On Mon, 3 Feb, 2025, 05:49 Pawani, ***@***.***> wrote: I actually did start working on this as you can see from my very first comment on this issue but no problem if @abhishekdubey369 <https://github.com/abhishekdubey369> has already made progress on this. @abhishekdubey369 <https://github.com/abhishekdubey369> Please let me know as to your preference. — Reply to this email directly, view it on GitHub <#39411 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AOY55UMNHW2I44DXWTQ3JBL2N2Y2BAVCNFSM6AAAAABWFCQV22VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMRZGY2DKMBTGI> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[pawani27]

@abhishekdubey369 Yeah sure.