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

Rate function added #17

Merged
merged 21 commits into from Jun 6, 2021
Merged

Rate function added #17

merged 21 commits into from Jun 6, 2021

Conversation

thsubaku9
Copy link
Contributor

This PR adds the Rate calculation function to go-financial (using Newton Rapson for the root calculation)

@thsubaku9
Copy link
Contributor Author

@gyanesh-m sorry to bother you but I think something is wrong with the CI. Do let me know when things are fixed so that I can continue with the PR

at the beginning (when = 1) or the end (when = 0) of each period
curRate: the rate compounded once per period rate
*/
func getRateRatio(pv, fv, pmt, curRate float64, nper int64, when paymentperiod.Type) float64 {
Copy link
Contributor

Choose a reason for hiding this comment

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

don't use float, use decimal. Check other functions in this file.

OpenDocument-formula-20090508.odt
*/

func Rate(pv, fv, pmt float64, nper int64, when paymentperiod.Type, params ...float64) (float64, bool) {
Copy link
Contributor

Choose a reason for hiding this comment

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

same, replace float with decimal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops sorry about that. But also glad to see this library now supports floating precession :)

@gyanesh-m
Copy link
Contributor

gyanesh-m commented Mar 21, 2021

@thsubaku9 The pipeline is fine. There are errors in golangci-lint check. Find them locally using make lint-check. Make sure you have golangci-lint installed. If not, use make install-lint to install it.

@thsubaku9
Copy link
Contributor Author

Let me know if any more changes are required @gyanesh-m

@gyanesh-m
Copy link
Contributor

@thsubaku9 will review the changes over weekend.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
reducing_utils.go Show resolved Hide resolved
reducing_utils.go Outdated Show resolved Hide resolved
@thsubaku9 thsubaku9 requested a review from gyanesh-m April 4, 2021 08:02
@thsubaku9
Copy link
Contributor Author

lets gooooooo @gyanesh-m

README.md Outdated Show resolved Hide resolved
@thsubaku9
Copy link
Contributor Author

Lets go ? @gyanesh-m

reducing_utils.go Outdated Show resolved Hide resolved
reducing_utils.go Outdated Show resolved Hide resolved
reducing_utils.go Outdated Show resolved Hide resolved
reducing_utils.go Outdated Show resolved Hide resolved
reducing_utils.go Outdated Show resolved Hide resolved
reducing_utils.go Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@thsubaku9
Copy link
Contributor Author

thsubaku9 commented May 28, 2021

Any updates here ? @krantideep95 @gyanesh-m @captn3m0

@gyanesh-m
Copy link
Contributor

Hey @thsubaku9, I will review this over the coming weekend.

Copy link
Contributor

@gyanesh-m gyanesh-m left a comment

Choose a reason for hiding this comment

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

lgtm

@gyanesh-m gyanesh-m merged commit 35a25fd into razorpay:master Jun 6, 2021
@gyanesh-m
Copy link
Contributor

@thsubaku9 Thank you for adding this to go-financial ! 💯 🎉

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

Successfully merging this pull request may close these issues.

None yet

2 participants