Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

parametrize tests #75

Merged
merged 2 commits into from
Nov 17, 2023
Merged

parametrize tests #75

merged 2 commits into from
Nov 17, 2023

Conversation

tal66
Copy link
Contributor

@tal66 tal66 commented Nov 15, 2023

hi, this closes #72.

@Kai-Striega
Copy link
Member

Kai-Striega commented Nov 16, 2023

Looks good to me! I've run the test suite to see if there are any problems.

There's one more improvement I think we can make. Notice how the values for Decimal and Float types are the same? i think we can parameterise across the dtype aswell. See the below test for an example:

@pytest.mark.parametrize('number_type', [Decimal, float])
@pytest.mark.parametrize('when', [0, 1, 'end', 'begin'])
def test_rate_with_infeasible_solution(self, number_type, when):
"""
Test when no feasible rate can be found.
Rate will return NaN, if the Newton Raphson method cannot find a
feasible rate within the required tolerance or number of iterations.
This can occur if both `pmt` and `pv` have the same sign, as it is
impossible to repay a loan by making further withdrawls.
"""
result = npf.rate(number_type(12.0),
number_type(400.0),
number_type(10000.0),
number_type(5000.0),
when=when)
is_nan = Decimal.is_nan if number_type == Decimal else numpy.isnan
assert is_nan(result)

Thank you for taking the time to contribute

@Kai-Striega Kai-Striega added this to the 2.0 milestone Nov 16, 2023
@Kai-Striega
Copy link
Member

@tal66 this looks good now and tests are green. I don't think there's much more that needs to be done. Are you happy for me to merge it?

@tal66
Copy link
Contributor Author

tal66 commented Nov 17, 2023

sure

@Kai-Striega Kai-Striega merged commit 671d2d7 into numpy:main Nov 17, 2023
13 checks passed
@Kai-Striega
Copy link
Member

Merged :) Thanks again for taking the time to contribute!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TST: Parameterise tests where needed
2 participants