Skip to content

bpo-35431: Drop the k <= n requirement - #13798

Merged
rhettinger merged 2 commits into
python:masterfrom
rhettinger:math_comb_zero_case
Jun 4, 2019
Merged

bpo-35431: Drop the k <= n requirement#13798
rhettinger merged 2 commits into
python:masterfrom
rhettinger:math_comb_zero_case

Conversation

@rhettinger

@rhettinger rhettinger commented Jun 4, 2019

Copy link
Copy Markdown
Contributor

@rhettinger
rhettinger requested review from mdickinson and tim-one June 4, 2019 08:18
@rhettinger
rhettinger merged commit 963eb0f into python:master Jun 4, 2019
@rhettinger
rhettinger deleted the math_comb_zero_case branch June 4, 2019 08:23
@mdickinson

Copy link
Copy Markdown
Member

LGTM. For simplicity, could we move the check for negative k in math_comb up nearer the top of the function? The current code path that leads to the "k must be non-negative" error isn't particularly obvious.

Comment thread Doc/library/math.rst
to the coefficient of k-th term in polynomial expansion of the
expression ``(1 + x) ** n``.

Raises :exc:`TypeError` if either of the arguments not integers.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo (predates this PR): missing "are" before "not integers". Same again below for math.perm. I'd personally reword as "... unless both arguments are integers".

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch.

DinoV pushed a commit to DinoV/cpython that referenced this pull request Jan 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants