binomial coefficient function #250
Conversation
|
there is already a function like this in |
|
true! I moved that one from compas.geometry._primitives.curves to compas.utilities.functions and fixed the imports |
tomvanmele
left a comment
tomvanmele
left a comment
There was a problem hiding this comment.
can you write a test for this one? should be a fairly easy one...
tests/utilities/test_functions.py
Outdated
|
|
||
| def test_binomial(): | ||
| assert binomial(3, 1) == 3 | ||
| assert binomial(21, 10) == 352716 |
There was a problem hiding this comment.
can you make sure to test the boundary cases?
for example, what if one or both of the number is zero?
see the geometry tests for setting up tests that are expected to fail.
also you can parametrize the input so the two assertions count as two tests...
tests/utilities/test_functions.py
Outdated
| pytest.param(2, -1, 0, marks=pytest.mark.xfail(raises=ValueError)), | ||
| ] | ||
| ) | ||
|
|
There was a problem hiding this comment.
no spacing here, because the stuff above is a decorator for the function and should be "attached" to it...
| :nosignatures: | ||
| fibonacci | ||
| binomial_coefficient |
There was a problem hiding this comment.
Shouldn't this be called binomial or the function renamed to binomial_coefficient? Or are these two different things?
There was a problem hiding this comment.
I think binomial_coefficient is the correct name.
There was a problem hiding this comment.
I agree, but what I meant is, you called the function binomial but referenced in the docs as binomial_coefficient, so doc will fail to find it. Unless, there's a second function called binomial_coefficient that I'm not seeing...
There was a problem hiding this comment.
see the warning in travis: https://travis-ci.com/compas-dev/compas/jobs/177444951#L658
There was a problem hiding this comment.
I am now using binomial_coefficient everywhere
Co-Authored-By: robin-oval <34314884+robin-oval@users.noreply.github.com>
…in-oval/compas into feature/binomial_coefficient
3 participants
No description provided.