Repository navigation
DOC: clarify pmt cash-flow signs - #157
Conversation
There was a problem hiding this comment.
Thanks for working on this, @naseweisssss !
The zero rate example is a great idea, it's much easier to do the math. I checked against the implementation, and it's right.
A few small requests:
- Keep "Future value" as the term for
fv. That is the terminology used throughout the API:pv,nper,ipmt,ppmt,rate- all document thefvparameter as “Future value”. Also,fvitself is defined as computing the future value. Renaming it only inpmtwould introduce an inconsistency. - Make the
pvnote perspective-independent, e.g., "Cash received is positive and cash paid out is negative. For a borrower, the loan amount is positive". ipmt,ppmt,nper, andfvshare the same convention and descriptions. A short note in the module docstring explaining the convention would be nice. Also could be a follow-up PR.
Thanks for the comments,
|
InessaPawson
left a comment
There was a problem hiding this comment.
Thanks for the quick turnaround, @naseweisssss! All requests from my review are addressed, so it’s Approve from me.
@stefanv I believe this is ready to be merged.
seberg
left a comment
There was a problem hiding this comment.
Thanks, this seems sensible to me and I don't have thoughts on bikeshedding things more. The one thing I feel more generally, is that using kwargs might be clearer, but they don't even have that nice names :).
Thanks, it doesnt look like I can merge this still. |
|
A maintainer needs to merge :). Thanks @naseweisssss! |
Add some documentations in relation to issues #130 to clarify the usage of signs on fv and pv