Skip to content

BUG: support DataFrame columns containing period arrays - #156

Open
sb123sb123 wants to merge 4 commits into
numpy:mainfrom
sb123sb123:fix-ipmt-pandas-period-arrays
Open

sb123sb123 wants to merge 4 commits into
numpy:mainfrom
sb123sb123:fix-ipmt-pandas-period-arrays

Conversation

@sb123sb123

Copy link
Copy Markdown

Summary

ipmt fails when its per input is a pandas Series of same-shaped period arrays and the other inputs are row-wise Series. Stack the period arrays into a row-by-period matrix, expand matching row parameters into columns, and broadcast them together. Apply the same normalization in ppmt and add regression coverage for both functions without adding a pandas test dependency.

Fixes #105.

Validation

  • pytest -q: 111 passed (Python 3.14, NumPy 2.5.3, pandas 3.0.6).
  • spin lint: Ruff, Pyright, and Mypy passed.
  • The issue reproduction passed on Python 3.11.4, NumPy 1.26.4, and pandas 2.2.3; the TestIpmt/TestPpmt group passed (38 tests). The source-only checkout lacked the compiled _cfinancial extension, so a temporary import stub was used for that focused test run and removed afterward.

AI assistance was used to prepare this change and its tests.

Comment thread numpy_financial/_financial.py Outdated

def _broadcast_payment_inputs(
rate: Any, per: Any, nper: Any, pv: Any, fv: Any, when: Any
) -> tuple[Any, Any, Any, Any, Any, Any]:

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.

Is there any point in adding the typing info when it is all Any?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

You were right: the six NDArray[Any] entries did not narrow the helper’s dynamic broadcast result. I removed the return annotation and kept the _ArrayLike input annotations. Ruff, Python 3.13.15 py_compile, and the focused nested-period regression test pass (1 passed). The source checkout lacks the compiled _cfinancial extension, so the focused test used a temporary import stub; this test path does not call that extension.

@sb123sb123

Copy link
Copy Markdown
Author

The revised head is 59c55de. The focused regression, Ruff, and Python 3.13 syntax checks pass locally. GitHub has created Test package and Type-check runs for this commit, but both are currently action_required and have not executed. Could a maintainer approve those two runs when convenient? I will address any resulting failures.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Development

Successfully merging this pull request may close these issues.

BUG: ipmt not working with numpy==1.26.4 when using a pandas df as input.

3 participants