-
-
Notifications
You must be signed in to change notification settings - Fork 82
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
Updating mirr docstring [issue 76] #86
Conversation
Hello @RVitalii, Thanks for taking the time to make a contribution! This looks good. There are just a few problems with the style guide. At the moment, the CI is failing because some lines are too long. Here is the output from the CI:
This means that on lines 940,964 and 967 you are exceeding the limit of 88 chars. Would you please shorten those lines and I'll take another look through it? |
Hello, I tried to update my forked-main branch from upstream and amend the original commit, but did a lot of mess instead. I didn't know that the iss76 branch is already connected to upstream, and everything I had to do is to update that branch. Eventually, I managed to push a new commit with the proposed changes: Please, let me know if I have to correct smth. Regards, |
There's still one check failing. See the Here's the failing output:
|
Hello @Kai-Striega, Updated. Here is the commit number: 4fa06ed Regards, |
@RVitalii thanks for contributing, this looks good now! Are there any last changes you would like to do before I merge it? |
@Kai-Striega, no I am fine. Thanks for support. |
Merged. Thanks again for taking the time to contribute. |
This pull request addresses issue #76 by enhancing the documentation for the
mirr
function.I have updated the docstring to provide clearer explanations of the function's purpose, parameters, and usage. Additionally, I incorporated a more detailed MIRR formula, including the reinvestment rate, and provided illustrative examples to guide users in understanding how to use the function effectively.