Python script to answer Rosalind test 2 (sum of two squares)

Question

The test: https://rosalind.info/problems/ini2/

Problem

Given: Two positive integers a and b, each less than 1000.
Return: The integer corresponding to the square of the hypotenuse of the right triangle whose legs have lengths a and b.

Sample Dataset

3 5

Sample Output

34

My script:

#!/bin/env python3
import argparse
import os.path
import sys

def int_sq(n):
    return int(n)**2

def square_hyp_triangle(a,b):
    #check int
    sq_sum=sum(int_sq(i) for i in [a,b]) 
    return sq_sum


def main(argv=None):
    parser = argparse.ArgumentParser(
            prog="intro",
            description='Import a file with integers')
    
    parser.add_argument('filename', 
                        type=argparse.FileType('r'), 
                        nargs='?',
                        help='an existing filename')

    args = parser.parse_args(argv)
    if not args.filename:
        sys.exit("Please provide an input file, or pipe it via stdin")

    for line in args.filename:
        line = line.rstrip().split()
        res = square_hyp_triangle(line[0],line[1])
        #print(line[0], line[1])
        print(res)
    #print(res)

if __name__ == '__main__':
    main() 

If you saved the above script named intro.py You can run it :

echo "3 5" | ./intro.py -

Which parts can be improved?

Answer 1

Imports are sorted -- nice!

---

optional type hinting

def int_sq(n):

Consider using this signature:

def int_sq(n: float) -> int:

(Recall that float includes int for hinting purposes.)

---

sum, map

    sq_sum = sum(int_sq(i) for i in [a, b]) 

Consider phrasing this as

    sq_sum = sum(map(int_sq, [a, b]))

since the temp var i is uninteresting and could be anonymous.

Also, delete the meaningless "#check int" comment. And maybe just return the expression without giving it a name.

---

docstring

The identifier square_hyp_triangle is fine. But do write an English sentence for it:

def square_hyp_triangle(a: float, b: float) -> float:
    """Finds the squared length of a triangle's hypotenuse.
    """

Consider writing another few lines so that $ python -m doctest *.py does an automated test to verify things work as intended.

    """Finds the squared length of a triangle's hypotenuse.

    >>> square_hyp_triangle(3, 5)
    34
    """

---

typer

In main the parser is nice enough, though "import" perhaps is a slightly odd verb. I'm glad you're using that library; it reduces the documentation burden for end users. Consider switching to typer, which gives you similar functionality "for free" (if you're using type hints).

I'm slightly surprised by the absence of ... , default=sys.stdin), but I guess your defaulting must work fine. The filename identifier seems to have the wrong name, given that you want it be an open file descriptor rather than a name.

In line.rstrip().split() maybe you had some CR/LF trouble? I don't see why you need to strip, given that split will discard whitespace for you.

---

tuple unpack

Avoid cryptic [0] / [1] subscripts, preferring a, b = line.split(). Or better, convert from str right here using:

        a, b = map(float, line.split())

Aha, now I understand that return int(n)**2 expression. That doesn't seem a very sensible place to do the conversion. I wouldn't really expect that a function could square "3". And even if it did, should I get back "9" instead of 9? Best to not even explore such territory, and stick to numeric values, so the squaring function has a single responsibility and is not also responsible for type conversions. Better to allocate that responsibility to the CLI interface called main.

Rejecting fractional inputs like 3.1 is maybe an unusual design choice. Guess it stems from the "Given:" and "Return:" parts of the spec, OK. Rosalind's "download a one-line text file dataset" requirement pushes this code in a slightly odd direction.

---

old debugs

        #print(line[0], line[1])

It served a purpose at one time. But now that your code is "done" and you're requesting review, it is the right time to tidy up and delete debug statements.

---

Which parts can be improved?

Adhere to Single Responsibility.

Recommend you adopt black auto-formatting, so expressions have a uniform appearance without even having to think about it.

Answer 2

The command-line usage is awkward. Your program needs two arguments: a and b. In a case like that, the easiest thing to do is configure argparse – no surprises here – to take two arguments. Instead, you have set things up so that the user must either put the two integers in a file and pass the file path as an argument or to use the piped-echo approach illustrated in your demo usage.

The awkward command-line usage also forces you to parse text. But that's all unnecessary if you just do the natural thing: take two arguments.

Whenever feasible, let argparse enforce the usage requirements. Your program requires a and b. There's no escaping that. Make the arguments required.

Whenever feasible, let argparse enforce data types. Same idea. Your program wants integers. Configure argparse with that detail and let the library do the work for you.

Move command line argument parsing out of main(). This is just a good habit to get into. It allows you to test/debug the command-line interface separately, if needed, and it keeps main() focused narrowly on its primary job: top-level program orchestration.

from argparse import ArgumentParser
import sys

def main(args):
    opts = parse_args(args)
    ss = sum_of_squares(opts.a, opts.b)
    print(ss)

def parse_args(args):
    ap = ArgumentParser()
    ap.add_argument('a', type = int)
    ap.add_argument('b', type = int)
    return ap.parse_args(args)

def sum_of_squares(*xs):
    return sum(x ** 2 for x in xs)

if __name__ == '__main__':
    main(sys.argv[1:])

Answer 3

• Don't parse command line arguments manually. Consider docopt instead.

• Usually I advocate a more functions, please approach. In this case it seems you went a bit too far. I see no benefits of wrapping operator ** into a function.

• int() may throw ValueError, which will terminate the script prematurely. Be prepared, and catch it.