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

Add fuzz test, property-based test, and table test for PV function #100

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -109,5 +109,9 @@ Thumbs.db
####################
poetry.lock

# hypothesis generated files #
##########################
/.hypothesis

# Things specific to this project #
###################################
49 changes: 27 additions & 22 deletions numpy_financial/_financial.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@
otherwise stated.
"""

from decimal import Decimal
import logging
from decimal import Decimal, DivisionByZero, InvalidOperation, Overflow
from typing import Literal, Union

import numba as nb
import numpy as np
Expand Down Expand Up @@ -511,33 +513,29 @@ def ppmt(rate, per, nper, pv, fv=0, when='end'):
return total - ipmt(rate, per, nper, pv, fv, when)


def pv(rate, nper, pmt, fv=0, when='end'):
def pv(
rate: Union[int, float, Decimal, np.ndarray],
Copy link
Author

@paulzuradzki paulzuradzki Jan 7, 2024

Choose a reason for hiding this comment

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

I added type hints here, but I see that they're not conventional in the repo. NP to remove the type hints if preferred for consistency; else, can keep it as optional.

Using Union since the package supports Python 3.9. Starting in Python 3.10, we can do int|float|Decimal|np.ndarray.

Copy link
Member

Choose a reason for hiding this comment

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

Type hints are great. Adding type hints is actually part of what I'll be working on once I have the broadcasting done.

nper: Union[int, float, Decimal, np.ndarray],
pmt: Union[int, float, Decimal, np.ndarray],
fv: Union[int, float, Decimal, np.ndarray] = 0,
when: Literal[0, 1, "begin", "end"] = "end",
):
"""Compute the present value.

Given:
* a future value, `fv`
* an interest `rate` compounded once per period, of which
there are
* `nper` total
* a (fixed) payment, `pmt`, paid either
* at the beginning (`when` = {'begin', 1}) or the end
(`when` = {'end', 0}) of each period

Return:
the value now

Parameters
----------
rate : array_like
Rate of interest (per period)
Required. The interest rate per period.
For example, use 6%/12 for monthly payments at 6% Annual Percentage Rate (APR).
nper : array_like
Number of compounding periods
Required. The total number of payment periods in an investment.
pmt : array_like
Payment
Required. The payment made each period. This does not change throughout the investment.
fv : array_like, optional
Future value
Optional. The future value or cash value attained after the last payment.
when : {{'begin', 1}, {'end', 0}}, {string, int}, optional
When payments are due ('begin' (1) or 'end' (0))
Optional. Indicates if payments are due at the beginning or end of the period
('begin' (1) or 'end' (0)). The default is 'end' (0).

Returns
-------
Expand Down Expand Up @@ -601,10 +599,17 @@ def pv(rate, nper, pmt, fv=0, when='end'):
"""
when = _convert_when(when)
(rate, nper, pmt, fv, when) = map(np.asarray, [rate, nper, pmt, fv, when])
temp = (1 + rate) ** nper
fact = np.where(rate == 0, nper, (1 + rate * when) * (temp - 1) / rate)
return -(fv + pmt * fact) / temp

try:
temp = (1 + rate) ** nper
fact = np.where(rate == 0, nper, (1 + rate * when) * (temp - 1) / rate)
return -(fv + pmt * fact) / temp

except (InvalidOperation, TypeError, ValueError, DivisionByZero, Overflow) as e:
paulzuradzki marked this conversation as resolved.
Show resolved Hide resolved
logging.error(f"Error in pv: {e}")
paulzuradzki marked this conversation as resolved.
Show resolved Hide resolved
return -0.0



# Computed with Sage
# (y + (r + 1)^n*x + p*((r + 1)^n - 1)*(r*w + 1)/r)/(n*(r + 1)^(n - 1)*x -
Expand Down
10 changes: 10 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ numba = "^0.58.1"


[tool.poetry.group.test.dependencies]
hypothesis = "^6.92.2"
Copy link
Member

Choose a reason for hiding this comment

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

I think adding hypothesis is the right idea here, however I don't have much experience with it. It might take me a while to thoroughly understand this

Copy link
Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Thanks! I've read the Hypothesis documentation and done some basic property based tests, but not much beyond that.

pytest = "^7.4"


Expand All @@ -60,3 +61,12 @@ ruff = "^0.1.6"
[tool.poetry.group.bench.dependencies]
asv = "^0.6.1"

[tool.pytest.ini_options]
filterwarnings = [
'ignore:.*invalid value encountered.*:RuntimeWarning',
'ignore:.*divide by zero encountered.*:RuntimeWarning',
'ignore:.*overflow encountered.*:RuntimeWarning'
]
markers = [
"slow: marks tests as slow (deselect with '-m \"not slow\"')",
]
165 changes: 161 additions & 4 deletions tests/test_financial.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
import math
from decimal import Decimal
from typing import Literal, Union

import hypothesis.strategies as st

# Don't use 'import numpy as np', to avoid accidentally testing
# the versions in numpy instead of numpy_financial.
import numpy
import pytest
from hypothesis import Verbosity, given, settings
from numpy.testing import (
assert_,
assert_allclose,
Expand All @@ -15,7 +19,6 @@

import numpy_financial as npf


class TestFinancial(object):
def test_when(self):
# begin
Expand Down Expand Up @@ -90,13 +93,167 @@ def test_decimal_with_when(self):


class TestPV:
Copy link
Author

Choose a reason for hiding this comment

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

fwiw, I would remove the test classes and rely on function naming convention (ex: test_pv*). Out of scope.

Copy link
Member

Choose a reason for hiding this comment

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

I agree on principle, however having them in classes makes it easier to run groups of tests for each function.

# Test cases for pytest parametrized example-based tests
test_cases = {
"default_fv_and_when": {
"inputs": {
"rate": 0.05,
"nper": 10,
"pmt": 1000,
},
"expected_result": -7721.73,
},
"specify_fv_and_when": {
"inputs": {
"rate": 0.05,
"nper": 10,
"pmt": 1000,
"fv": 0,
"when": 0,
},
"expected_result": -7721.73,
},
"when_1": {
"inputs": {
"rate": 0.05,
"nper": 10,
"pmt": 1000,
"fv": 0,
"when": 1,
},
"expected_result": -8107.82,
},
"when_1_and_fv_1000": {
"inputs": {
"rate": 0.05,
"nper": 10,
"pmt": 1000,
"fv": 1000,
"when": 1,
},
"expected_result": -8721.73,
},
"fv>0": {
"inputs": {
"rate": 0.05,
"nper": 10,
"pmt": 1000,
"fv": 1000,
},
"expected_result": -8335.65,
},
"negative_rate": {
"inputs": {
"rate": -0.05,
"nper": 10,
"pmt": 1000,
"fv": 0,
},
"expected_result": -13403.65,
},
"rates_as_array": {
"inputs": {
"rate": numpy.array([0.010, 0.015, 0.020, 0.025, 0.030, 0.035]),
"nper": 10,
"pmt": 1000,
"fv": 0,
},
"expected_result": numpy.array(
[-9471.30, -9222.18, -8982.59, -8752.06, -8530.20, -8316.61]
),
},
}

# Randomized input strategies for fuzz tests & property-based tests
Copy link
Author

Choose a reason for hiding this comment

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

Providing a "strategy" limits the values that the fuzzer (Hypothesis) will supply. The strategies that we re-use are defined here.

numeric_strategy = st.one_of(
st.decimals(),
st.floats(),
st.integers(),
)

when_period_strategy = st.sampled_from(["end", "begin", 1, 0])

def test_pv(self):
assert_almost_equal(npf.pv(0.07, 20, 12000, 0), -127128.17, 2)

def test_pv_decimal(self):
assert_equal(npf.pv(Decimal('0.07'), Decimal('20'), Decimal('12000'),
Decimal('0')),
Decimal('-127128.1709461939327295222005'))
assert_equal(
npf.pv(Decimal("0.07"), Decimal("20"), Decimal("12000"), Decimal("0")),
Decimal("-127128.1709461939327295222005"),
)

@pytest.mark.parametrize("test_case", test_cases.values(), ids=test_cases.keys())
Copy link
Author

Choose a reason for hiding this comment

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

ids= paramater gets us test case labels.

With ids

$ pytest tests/test_financial.py::TestPV::test_pv_examples -vv
tests/test_financial.py::TestPV::test_pv PASSED                                                                             [  9%]
tests/test_financial.py::TestPV::test_pv_decimal PASSED                                                                     [ 18%]
tests/test_financial.py::TestPV::test_pv_examples[default_fv_and_when] PASSED                                               [ 27%]
tests/test_financial.py::TestPV::test_pv_examples[specify_fv_and_when] PASSED                                               [ 36%]
tests/test_financial.py::TestPV::test_pv_examples[when_1] PASSED                                                            [ 45%]
tests/test_financial.py::TestPV::test_pv_examples[when_1_and_fv_1000] PASSED                                                [ 54%]
tests/test_financial.py::TestPV::test_pv_examples[fv>0] PASSED                                                              [ 63%]
tests/test_financial.py::TestPV::test_pv_examples[negative_rate] PASSED                                                     [ 72%]
tests/test_financial.py::TestPV::test_pv_examples[rates_as_array] PASSED                                                    [ 81%]
tests/test_financial.py::TestPV::test_pv_fuzz PASSED                                                                        [ 90%]
tests/test_financial.py::TestPV::test_pv_interest_rate_sensitivity PASSED                                                   [100%]

Without ids (no labels)

$ pytest tests/test_financial.py::TestPV::test_pv_examples -vv
tests/test_financial.py::TestPV::test_pv_examples[test_case0] PASSED                                                        [ 27%]
tests/test_financial.py::TestPV::test_pv_examples[test_case1] PASSED                                                        [ 36%]
tests/test_financial.py::TestPV::test_pv_examples[test_case2] PASSED                                                        [ 45%]
tests/test_financial.py::TestPV::test_pv_examples[test_case3] PASSED                                                        [ 54%]
tests/test_financial.py::TestPV::test_pv_examples[test_case4] PASSED                                                        [ 63%]
tests/test_financial.py::TestPV::test_pv_examples[test_case5] PASSED                                                        [ 72%]
tests/test_financial.py::TestPV::test_pv_examples[test_case6] PASSED                                                        [ 81%]

def test_pv_examples(self, test_case):
inputs, expected_result = test_case["inputs"], test_case["expected_result"]
result = npf.pv(**inputs)
assert result == pytest.approx(expected_result)

@pytest.mark.slow
@given(
rate=numeric_strategy,
nper=numeric_strategy,
pmt=numeric_strategy,
fv=numeric_strategy,
when=when_period_strategy,
)
@settings(verbosity=Verbosity.verbose)
def test_pv_fuzz(
self,
rate: Union[int, float, Decimal, numpy.ndarray],
nper: Union[int, float, Decimal, numpy.ndarray],
pmt: Union[int, float, Decimal, numpy.ndarray],
fv: Union[int, float, Decimal, numpy.ndarray],
when: Literal[0, 1, "begin", "end"],
) -> None:
npf.pv(rate, nper, pmt, fv, when)
paulzuradzki marked this conversation as resolved.
Show resolved Hide resolved

@pytest.mark.slow
@given(
rate=st.floats(),
nper=st.floats(),
pmt=st.floats(),
fv=st.floats(),
when=when_period_strategy,
)
@settings(verbosity=Verbosity.verbose)
def test_pv_time_value_of_money(
self,
rate: float,
nper: float,
pmt: float,
fv: float,
when: Literal[0, 1, "begin", "end"],
) -> None:
"""
Test that the present value is inversely proportional to number of periods,
all other things being equal.
"""
npf.pv(rate, nper, pmt, fv, when) > npf.pv(
rate, float(nper) + float(1), pmt, fv, when
)

@pytest.mark.slow
@given(
rate=st.floats(),
nper=st.floats(),
pmt=st.floats(),
fv=st.floats(),
when=when_period_strategy,
)
@settings(verbosity=Verbosity.verbose)
def test_pv_interest_rate_sensitivity(
self,
rate: float,
nper: float,
pmt: float,
fv: float,
when: Literal[0, 1, "begin", "end"],
) -> None:
"""
Test that the present value is inversely proportional to the interest rate,
all other things being equal.
"""
npf.pv(rate, nper, pmt, fv, when) > npf.pv(rate + 0.1, nper, pmt, fv, when)


class TestRate:
Expand Down