Solving roots of negative numbers

Question

As part of an assignment for college summer classes, I was tasked with writing a program that solves roots of negative numbers. I've ran it through the test cases I was provided and it computes correctly. I'm looking for any feedback, especially about the determine_factors function, which really should be renamed. It looks messy and I don't like it, but it's all I could come up with. I'll be tweaking it since it's not due for a while, but I wanted to get some experienced eyes on this.

"""
Helper tool for solving negative square roots.
@author Ben Antonellis
@date 06-08-2020
"""
from math import isqrt
from typing import List, Union, Tuple
def is_perfect(number: int) -> bool:
 """
 Returns if a number is a perfect square.
 """
 return number == isqrt(number) ** 2
def get_factors(number: int) -> List[List[int]]:
 """
 Returns all the factors of a number.
 """
 factors = [[x, number // x] for x in range(2, number + 1) if number % x == 0]
 factors = factors[:len(factors) // 2] # Removes duplicates and any [1, x] pairs #
 return factors
def determine_factors(factors: List[int], num: int) -> str:
 """
 Determines what factors are left at the end of calculation.
 """
 simplified, remainder = 0, 0
 for number, times_in_number in factors:
 if number == times_in_number:
 return f"{number}i"
 if get_factors(number) == [] and get_factors(times_in_number) == []:
 return f"√({num})i"
 if is_perfect(number):
 simplified += isqrt(number)
 if not is_perfect(times_in_number):
 if get_factors(times_in_number) == [] or not any(is_perfect(x) or is_perfect(y) for [x, y] in get_factors(times_in_number)):
 remainder += times_in_number
 return f"{simplified}√({remainder})i"
 elif is_perfect(times_in_number):
 simplified += isqrt(times_in_number)
 if not is_perfect(number):
 if get_factors(number) == []:
 remainder += number
 return f"{simplified}√({remainder})i"
 if not any(is_perfect(number) or is_perfect(times_in_number) for number, times_in_number in factors):
 return f"√({num})i"
def solve(num: int) -> str:
 """
 Solves a negative square root with the passed number.
 """
 num = abs(num)
 # Check for prime numbers #
 if not any(num % i == 0 for i in range(2, num)):
 return f"√({num})i"
 # Find all factors #
 factors = get_factors(num)
 # Determine what factors to use and weed out perfect numbers #
 return determine_factors(factors, num)
def main():
 num = int(input("Enter negative inside square root: "))
 result = solve(num)
 print(result)
if __name__ == "__main__":
 main()

Test cases are below

Input -> Output
---------------
-52 -> 2√(13)i
-20 -> 2√(5)i
-35 -> √(35)i
-36 -> 6i
-30 -> √(30)i
-18 -> 3√(2)i
-70 -> √(70)i
-90 -> 3√(10)i
-100 -> 10i

Answer 1

Style

You've done a great job of style from a technical point of view.

• You're following PEP-8 guidelines.
• The functions have type-hints
• The functions have docstrings

Defects:

• The docstrings aren't very useful:

  >>> help(get_factors)
  Help on function get_factors in module __main__:
  get_factors(number: int) -> List[List[int]]
   Returns all the factors of a number.
  

  This doesn't explain what the factors are going to be. A list of list of integers, yes, but no clue what that means, or to expect that [1, x] will be excluded.

• the determine_factors method is a wall of code, with not a single comment insight describing how it is supposed to work.

is_perfect()

This is actually pretty close to perfect. :-)

get_factors()

You are (without any indication to the caller) omitting [1, x] (should be at least mentioned in the docstring.

You are finding all factors from 2 up to (and including!) number, which means the last pair generated is [number, 1]. Why do you skip generating [1, number] by starting at 2, yet continue up to number + 1?

How does factors[:len(factors) // 2] remove duplicates, when it is using truncating integer division, and you can have an odd number of factor pairs?

Why bother generating the factor pairs if you are just going to discard half of them anyway? Just iterate from 2 up to (and including) isqrt(number). Way more efficient.

determine_factors()

I have no idea how this function works. simplified += ... and remainder += ...? Multiplication of factors is not additive, so this doesn't make sense.

Moreover, the function doesn't work. -54 and -96 return nothing:

>>> solve(-54)
>>> solve(-96)

A better algorithm

Instead of computing all of the factors of number, determine the prime factorization of number. For instance:

$$ \sqrt {96} = \sqrt { 2^5 * 3^1 } $$

Any odd power will result in that factor to the power of 1 under the square root. After removing those, you've got primes raised to even powers, which can all be divided by two, and moved out from the square-root:

$$ \sqrt {96} = \sqrt { 2^4 } * \sqrt {2^1 * 3^1} = 2^2 * \sqrt {2^1 * 3^1} $$