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 Root and RootRound methods #153

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mediocregopher
Copy link

For computing the arbitrary nth root of any number. I also go formatted some unrelated bits of code.

I implemented the shifting nth root algorithm in order to compute an nth root to an arbitrary precision. Other methods, such as Newton's method, are potentially faster but involve some amount of uncertainty as they only give an approximation, while shifting nth root gives the precise answer. Also, a slow method is better than no method :P

@mwoss
Copy link
Member

mwoss commented Mar 30, 2021

Sorry for the late response, really late response :D
I like your implementation, it's quite clear and understandable. Also, those unit tests are implemented quite well too imo :3!
One thing, I've noticed this implementation is only working on integers, it's not ideal, but I think it is fine as long it is documented tho. I will try to come up with a more flexible solution, but for now, let's go with your solution. Could you rebase to master? :3

@mediocregopher
Copy link
Author

Hi @mwoss , thanks for taking a look at the PR :) I will try to find time one of these days to clean it up as you ask but tbh it may be a while. It might be faster for you to simply take over the the PR or open a new one (I don't really care if I get contributor credit or not).

@mwoss
Copy link
Member

mwoss commented Apr 15, 2021

Sure, I can do it myself too. For sure I will at least put your GH nickname in the changelog, as I really appreciate your contribution to this library :3

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.

2 participants