Skip to content

rational-numbers missing test case for negative rational exponent #1909

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

Closed
neduard opened this issue Jan 6, 2022 · 3 comments · Fixed by #1914
Closed

rational-numbers missing test case for negative rational exponent #1909

neduard opened this issue Jan 6, 2022 · 3 comments · Fixed by #1914

Comments

@neduard
Copy link
Contributor

neduard commented Jan 6, 2022

The problem description states:

Exponentiation of a rational number r = a/b to a negative integer power n is r^n = (b^m)/(a^m), where m = |n|.

However this is not checked for in the tests.

As such, the tests pass even if the implementation does something like

def rational_exponentiation(r, n):
  return Fraction(r.a ** n, r.b ** n)

Would my understanding be correct? I'm happy to raise PR and add a test, just that I don't know how to generate UUID for it 😅

(thanks @angelikatyborska for suggesting raising an issue)

@ericbalawejder
Copy link

It appears that there are only two tests for this case and they both use a positive integer power. No negative integer power as you mentioned. It would seem valuable to create a test implementation for it.

Even though it is not stated in the problem description, there is also no test for taking a negative real number and raising it to a rational power.

Exponentiation of a real number x to a rational number r = a/b is x^(a/b) = root(x^a, b), where root(p, q) is the qth root of p.

If x were to be less than zero (negative), and r = a/b, where b is even, this would result in complex roots. If b is odd, it is ok to take the odd roots of negative numbers, as it preserves the sign. It may be a valuable lesson when implementing this particular method/function. Thoughts?

@petertseng
Copy link
Member

Correct understanding. Adding a test for negative integer power would be good.

A UUID may be generated with any tool of your choice that generates version 4 UUIDs, even those that are available as a webpage on the internet.

For consideration of negative real numbers raised to rational powers, I suggest a separate issue.

@neduard
Copy link
Contributor Author

neduard commented Jan 8, 2022

Thanks @ericbalawejder and @petertseng ! I've created PR #1914 just for the negative integer tests.

@SaschaMann SaschaMann linked a pull request Jan 9, 2022 that will close this issue
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 a pull request may close this issue.

3 participants