6
\$\begingroup\$

https://github.com/CascadeIllusion/DesmOEIS

Recently began work on a Python project called DesmOEIS to build up my portfolio. It's a simple console program that looks up integer sequences from the OEIS, parses them, and converts them into Desmos lists using the Desmos API.

The program works fine, I just want general feedback to make sure I'm doing things the right way (not just code-wise, but also in terms of project structuring). Unit tests succeeded on my end, but then again maybe I'm not building good unit tests.

console.py

import sys
import webbrowser
from parsing import *
from desmos import *
from sequence import Sequence


def main():

    intro = \
      "DesmOEIS\n" \
      "A tool for converting OEIS sequences to Desmos lists.\n" \
      "\n" \
      "Type id=<OEIS id> (without the brackets) to convert a sequence. \n" \
      "Type help for a list of all valid commands. \n" \
      "Type exit to close the application. \n" \

    help = \
        "\nSyntax: (Command Name)=(Argument)\n\n" \
        "id: Attempts to convert an OEIS id argument into a Desmos list. \n" \
        "The \"A\" is optional, and trailing zeros may be excluded.\n\n" \
        \
        "name: Assigns the resulting Desmos list to a variable with the given name. \n" \
        "Names must be exactly one letter character (except \"e\"), no numbers or special characters.\n\n" \
        \
        "trim: Filters a list using Python-style slicing syntax. For A:B:C:\n" \
        "A is the starting index (inclusive), default 0.\n" \
        "B is the ending index (exclusive), default is the list length.\n" \
        "C is a step value that is used to skip every C elements, default is 1 (don't skip anything).\n\n" \
        \
        "ext: Pass Y to this to output the extended version of the OEIS sequence.\n" \
        "WARNING: Passing an entire extended sequence this way is usually not a good idea, as such\n" \
        "sequences can be hundreds of elements long, and can cause your browser to hang. You may want\n" \
        "to combine this with trimming syntax to reduce the number of elements.\n\n" \
        \
        "view: Opens the .html file containing the last converted sequence since starting the program. \n" \
        "Does not work if used before converting a sequence.\n\n" \
        \
        "help: View a list of all valid commands.\n\n" \
        \
        "exit: Closes the application." \

    print(intro)

    file = None

    while True:
        cmd = input()

        if cmd == "help":
            print(help)
            continue

        if cmd == "view":
            if file is None:
                print("No sequence converted yet.")
                continue
            webbrowser.open(f"file://{os.path.realpath(file)}")
            continue

        if cmd == "exit":
            sys.exit()

        # Multiple commands are comma separated
        cmds = cmd.split(', ')
        cmds[-1] = cmds[-1].replace(',', '')

        if not cmds[0].startswith("id"):
            print("First argument must be id.")
            continue

        args = dict()

        for i in cmds:
            i = i.split("=")

            cmd = i[0]
            value = i[1]

            args[cmd] = value

        id = parse_id(args)

        results = find_id(id)

        if results:
            sequence = Sequence(id)
            sequence.args = args
            sequence.results = results
        else:
            print("Invalid id.")
            continue

        sequence.integers = parse_integers(sequence)

        name = sequence.args.get("name")

        if name:
            if len(name) > 1:
                print("Variable names must be one character only.")
                continue

            if str.isdecimal(name) or name == 'e':
                print("Numeric names and the constant e (2.71828...) are not allowed.")
                continue

        sequence.name = name

        file = create_expression(sequence, create_desmos_list)

        print("Sequence converted successfully! \n")


if __name__ == '__main__':
    main()

parsing.py

import requests


def parse_id(args):

    id = args.get("id")

    # Remove the preceding A if included
    id = id.replace('A', '')

    # Add trailing zeros if necessary
    length = len(id)
    if length < 6:
        for i in range(0, 6 - length):
            id = "0" + id

    # Add A at the beginning of the query
    id = 'A' + id

    return id


def find_id(id):

    url = f"https://oeis.org/search?q=id:{id}&fmt=text"
    r = requests.get(url)

    if "No results." in r.text:
        return None

    return r


def parse_integers(sequence):

    text = sequence.results.text

    text = str.splitlines(text)

    rows = []

    if sequence.args.get("ext") == "Y":

        b_id = sequence.id.replace('A', 'b')
        url = f"https://oeis.org/{sequence.id}/{b_id}.txt"
        r = requests.get(url)
        sequence.results = r
        text = r.text
        text = str.splitlines(text)

        for line in text:
            space = line.find(" ")
            row = line[space + 1:]
            row = row.split(', ')
            rows.append(row)

    else:

        for line in text:
            if line.startswith('%S') or line.startswith('%T') or line.startswith('%U'):
                # integers start 11 characters into the line
                row = line[11:]
                row = row.split(',')
                rows.append(row)

    rows = [row for integer in rows for row in integer]

    # Remove empty elements resulting from commas at the end of the %S and %T rows
    rows = list(filter(None, rows))

    trim = sequence.args.get("trim")

    if trim:
        if ":" not in trim:
            print("Trim argument missing colons ( : ).")
            return
        trim = trim.split(":")
        if not (trim[0] is "" or trim[1] is ""):
            for i in trim:
                if i.isdigit() and trim[0] >= trim[1]:
                    print("Start value must be less than the end value.")
                    return
        if trim[0] is "":
            trim[0] = '0'
        if trim[1] is "":
            trim[1] = len(rows)
        for i in trim:
            i = str(i)
            if not i.isdigit():
                print("Invalid input for trim argument.")
                return
        trim = list(map(int, trim))
        start = trim[0]
        end = trim[1]
        if len(trim) == 3:
            if trim[2] is 0:
                print("Step value cannot be zero.")
                return
            step = trim[2]
            rows = rows[start:end:step]
        else:
            rows = rows[start:end]

    return rows

desmos.py

import os
from sequence import Sequence

def create_expression(sequence, func):

    graph = open("../resources/desmos_graph_base.html")
    graph = graph.read()

    expr = func(sequence)

    # Add another placeholder comment below the expression to allow for further expressions
    graph = graph.replace("<!-- PLACEHOLDER -->", f"{expr} \n <!-- PLACEHOLDER -->")

    sequence.graph = graph

    dir = "../graphs/"
    if not os.path.exists(dir):
        os.makedirs(dir)
    filename = f"{dir}{sequence.id}.html"
    out_graph = open(filename, "w")
    out_graph.write(graph)

    return filename


def create_desmos_list(sequence):

    integers = sequence.integers

    desmos_list = str(integers)
    desmos_list = desmos_list.replace("'", "")

    name = sequence.name

    if sequence.args.get("name") is not None:
        name = name + "="
    else:
        name = ""

    expr = f"calculator.setExpression({{ id: 'graph1', latex:\"{name}{desmos_list}\" }});"
    return expr

sequence.py

class Sequence():

    def __init__(self, id):
        self._id = id

    @property
    def id(self):
        return self._id

    @property
    def args(self):
        return self._args

    @args.setter
    def args(self, args):
        self._args = args

    @property
    def integers(self):
        return self._integers

    @integers.setter
    def integers(self, integers):
        self._integers = integers

    @property
    def results(self):
        return self._results

    @results.setter
    def results(self, results):
        self._results = results

    @property
    def name(self):
        return self._name

    @name.setter
    def name(self, name):
        self._name = name

test_parsing.py

import unittest
from parsing import *
from sequence import *

"""
# Example sequences used:
http://oeis.org/A000045
http://oeis.org/A000047
http://oeis.org/A139827
"""


class TestIdParsing(unittest.TestCase):

    def test_parse_id(self):
        args = {"id": "A000045"}
        self.assertEqual("A000045", parse_id(args))

    def test_parse_id_no_prefix(self):
        args = {"id": "000045"}
        self.assertEqual("A000045", parse_id(args))

    def test_parse_id_no_trailing_zeros(self):
        args = {"id": "A45"}
        self.assertEqual("A000045", parse_id(args))

    def test_parse_id_no_trailing_zeros_no_prefix(self):
        args = {"id": "45"}
        self.assertEqual("A000045", parse_id(args))

    def test_parse_id_six_digit(self):
        args = {"id": "A139827"}
        self.assertEqual("A139827", parse_id(args))

    def test_parse_id_six_digit_no_prefix(self):
        args = {"id": "139827"}
        self.assertEqual("A139827", parse_id(args))


class TestIdFinding(unittest.TestCase):

    def test_find_id_success(self):
        id = "A000045"
        self.assertNotEqual(None, find_id(id))

    def test_find_id_fail(self):
        id = "A123ABC"
        self.assertEqual(None, find_id(id))


class TestIntegerParsing(unittest.TestCase):

    def test_parse_integers(self):

        args = {"": ""}

        id = "A000045"

        integers = [0, 1, 1, 2, 3, 5, 8, 13, 21, 34, 55, 89, 144, 233, 377, 610, 987, 1597, 2584, 4181, 6765, 10946,
                    17711, 28657, 46368, 75025, 121393, 196418, 317811, 514229, 832040, 1346269, 2178309, 3524578,
                    5702887, 9227465, 14930352, 24157817, 39088169, 63245986, 102334155]

        integers = list(map(str, integers))

        url = f"https://oeis.org/search?q=id:{id}&fmt=text"
        r = requests.get(url)

        sequence = Sequence(id)
        sequence.integers = integers
        sequence.results = r
        sequence.args = args

        self.assertEqual(integers, parse_integers(sequence))

    # Use a different sequence because most extended sequences are too big to reasonably fit
    def test_parse_integers_ext(self):

        args = {"ext": "Y"}

        id = "A000047"

        integers = [1, 2, 3, 5, 8, 15, 26, 48, 87, 161, 299, 563, 1066, 2030, 3885, 7464, 14384, 27779, 53782, 104359,
                    202838, 394860, 769777, 1502603, 2936519, 5744932, 11249805, 22048769, 43248623, 84894767,
                    166758141, 327770275, 644627310, 1268491353, 2497412741, 4919300031, 9694236886, 19112159929]
        integers = list(map(str, integers))

        url = f"https://oeis.org/search?q=id:{id}&fmt=text"
        r = requests.get(url)

        sequence = Sequence(id)
        sequence.integers = integers
        sequence.results = r
        sequence.args = args

        self.assertEqual(integers, parse_integers(sequence))


if __name__ == '__main__':
    unittest.main()
\$\endgroup\$
1
  • 2
    \$\begingroup\$ Great first question! \$\endgroup\$
    – Reinderien
    Jul 7, 2020 at 18:50

1 Answer 1

4
\$\begingroup\$

Line continuations

Multi-line strings like this:

intro = \
  "DesmOEIS\n" \
  "A tool for converting OEIS sequences to Desmos lists.\n" \
  "\n" \
  "Type id=<OEIS id> (without the brackets) to convert a sequence. \n" \
  "Type help for a list of all valid commands. \n" \
  "Type exit to close the application. \n" \

are better-represented as parenthesized expressions:

intro = (
    "DesmOEIS\n" 
    "A tool for converting OEIS sequences to Desmos lists.\n" 
    "\n" 
    "Type id=<OEIS id> (without the brackets) to convert a sequence.\n" 
    "Type help for a list of all valid commands.\n" 
    "Type exit to close the application.\n" 
)

Also note that the second level of indentation should be at four spaces and not two.

Consider case-insensitive commands

It would be easy, and a user quality-of-life improvement, to compare strings against lowered input:

cmd = input().lower()

If you want to get really fancy you could implement unambiguous prefix string matching, i.e. hel would match help, but that is more advanced.

List parsing

Rather than this:

    # Multiple commands are comma separated
    cmds = cmd.split(', ')

it would be safer to split only on a comma, and then strip every resulting entry.

Dictionary literals

args = dict()

can be

args = {}

Unpacking

        i = i.split("=")

        cmd = i[0]
        value = i[1]

can be

cmd, value = i.split('=')

Replace or ltrim

# Remove the preceding A if included
id = id.replace('A', '')

does not do what you say it does. It replaces 'A' anywhere in the string. Instead, consider lstrip('A') which removes any number of 'A' from the left, or if you want to be more precise, use regular expressions and '^A'.

Character repetition

    for i in range(0, 6 - length):
        id = "0" + id

can be

id = '0'*(6 - length) + id

Requests parameters

url = f"https://oeis.org/search?q=id:{id}&fmt=text"
r = requests.get(url)

should use a dict argument to params instead of baking those into the URL; read https://2.python-requests.org/en/master/user/quickstart/#passing-parameters-in-urls

Requests checking for failure

Call r.raise_for_status() after your get. Otherwise the failure mode for HTTP requests will be hidden.

Pathlib

Consider the pathlib.Path alternatives to these calls:

if not os.path.exists(dir):
    os.makedirs(dir)

Setters and getters

This kind of boilerplate:

@property
def args(self):
    return self._args

@args.setter
def args(self, args):
    self._args = args

is somewhat unpythonic. _args is not a strictly private variable by anything other than convention. You are not gaining anything from these properties. Delete them and rename _args to args if you're going to let users modify this member.

Unit tests

Great; you have some! Consider mocking away requests.get in your tests; otherwise this could not strictly be considered a unit test.

\$\endgroup\$
4
  • 2
    \$\begingroup\$ id is a python builtin, so it shouldn't be used as a variable name. Padding the id with leading zeros can be done with an f-string: f"{seq_id:0>6s}". \$\endgroup\$
    – RootTwo
    Jul 7, 2020 at 19:51
  • \$\begingroup\$ @RootTwo I've never once used id, the only time I've seen it used is to explain how is is different to ==. \$\endgroup\$
    – Peilonrayz
    Jul 7, 2020 at 20:44
  • 1
    \$\begingroup\$ Wow, thanks a bunch for your feedback! I'm really glad I asked; I didn't know about some of the syntax tricks like with the unpacking or the character repetition. Also did not know about the request parameters either. Very, very much appreciated! \$\endgroup\$ Jul 7, 2020 at 21:59
  • \$\begingroup\$ @Peilonrayz it was in the original code. I was adding a comment to Reinderien's answer. \$\endgroup\$
    – RootTwo
    Jul 7, 2020 at 22:01

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.