9
\$\begingroup\$

A python program made with a friend to draw n dimensional hyper-cubes in python using turtle. Any suggestions for improvement.

import turtle
from turtle import *
import time
import math

n = int(input("How many dimensions?: "))
length = int(input("How long is each side?: "))

angle = (math.pi/180)*float(input("Angle: "))
offsetx = int(input("offset of the x:"))
offsety = int(input("offset of the y:"))

vertices = []
numlist = []
t = turtle.Turtle()
t.penup()
t.shape('circle')
t.shapesize(0.01)
t.speed(0)
tot = 0
r = 0


for i in range(0,2**n):
    temp = list(bin(i))
    temp.remove("b")
    temp.remove("0")
    while len(temp) < n:
        temp.insert(0,"0")
    vertices.insert(0,temp)

def Vertex(vertex):
    import math
    global t
    y = 0
    x = 0
    if vertex[0] == "1":
        y = y - length

    for i in range(1,n):
        if vertex[i] == "1":

            y = y+(math.sin(((angle)/((n-1)/(n-i)))-math.pi/2))*length
            x = x+(math.cos(((angle)/((n-1)/(n-i)))-math.pi/2))*length
    t.goto(x+offsetx,y+offsety)
    t.dot(3)

for i in vertices:
    print("DRAWING VERTICES,",int((r/(2**n))*100),"% COMPLETE")
    Vertex(i)

    r = r + 1

r = 0

while len(vertices) > 0:
    x = vertices[0]
    for y in vertices:
        for i in range(0,n):
            if x[i] == y[i]:
                tot = tot+1
        if tot == n-1:
            Vertex(x)
            t.pendown()
            Vertex(y)
            t.penup()
        tot = 0
    r = r+1
    print("DRAWING SIDES",int((r/(2**n))*100), "% COMPLETE")
    vertices.remove(x)

print("HYPERCUBE DRAWN")                           
nope = input("")
\$\endgroup\$
10
\$\begingroup\$

Generation of vertices

You could use itertools.product to generate vertices. You can replace:

for i in range(0,2**n):
    temp = list(bin(i))
    temp.remove("b")
    temp.remove("0")
    while len(temp) < n:
        temp.insert(0,"0")
    vertices.insert(0,temp)

with:

import itertools
vertices = list(itertools.product('10', repeat=n))

Import math

import math should probably be at the top of the file as per the Python code style called PEP 8:

Imports are always put at the top of the file, just after any module comments and docstrings, and before module globals and constants.

from xxx import *

The import * syntax is usually frowned upon as it mess with your namespace. In your case, it doesn't even seem to be needed so you can easily get rid of it.

Loop like a native

I highly recommand Ned Batchelder's excellent talk called "Loop Like A Native". Among other things, it tells you that in general, you do not need to try to keep track of the index as you iterate over something (which is what you are trying to do with r = r+1 (which can be written r += 1)). A buitlin called enumerate exists to help you in this task.

Thus, you could replace:

r = 0
for i in vertices:
    print("DRAWING VERTICES,",int((r/(2**n))*100),"% COMPLETE")
    Vertex(i)
    r = r + 1

with

for i, v in enumerate(vertices):
    print("DRAWING VERTICES,",int((i/(2**n))*100),"% COMPLETE")
    Vertex(v)

(I took this chance to perform the renaming of the variable)

Variable definition

I find it a good habit to define variable as late as possible and in the smallest possible scope. In your case, instead of having tot defined at the very top of your file and then used in a triply-nested loop only to be reset at the end of the inner loop, it is probably a good idea to define it in the relevant place so that you don't even need to re-set it at the end of the iteration. You'd get something like:

    tot = 0
    for i in range(n):
        if x[i] == y[i]:
            tot = tot+1
    if tot == n-1:
        Vertex(x)
        t.pendown()
        Vertex(y)
        t.penup()

Also, using this technique, you'd detect easily unused variable like numlist.

Rewriting angles computations

In :

        y = y+(math.sin(((angle)/((n-1)/(n-i)))-math.pi/2))*length
        x = x+(math.cos(((angle)/((n-1)/(n-i)))-math.pi/2))*length

you could:

  • use the += operator
  • move the multiplication by length to the beginning of the expression
  • use a temporary variable to hold the complicated value computed twice
  • get rid of useless parenthesis
  • add a bit of space

And you'd get something like:

    if vertex[i] == "1":
        tmp_angle = (angle / ((n-1)/(n-i))) - math.pi/2
        y += length * math.sin(tmp_angle)
        x += length * math.cos(tmp_angle)

itertools to the rescue again

Your last iteration over vertices removing elements as you go is quite complicated and could be made clearer with a call to itertools.

for x, y in itertools.combinations(vertices, 2):
    tot = 0
    for i in range(n):
        if x[i] == y[i]:
            tot = tot+1
    if tot == n-1:
        Vertex(x)
        t.pendown()
        Vertex(y)
        t.penup()

Iterating over 2 arrays at once

Once again, you'll find more about this in the talk mentioned above but there is a better way to iterate over 2 arrays at once : you could use zip.

sum the Python way

Once again, instead of keeping track of the number of equal pairs, you could use the sum builtin.

for x, y in itertools.combinations(vertices, 2):
    if sum(xi == yi for xi, yi in zip(x, y)) == n-1:
        Vertex(x)
        t.pendown()
        Vertex(y)
        t.penup()

Correct data type

Instead of using literal strings 10 and 1, we could use a more relevant data type like booleans.

Separation of concern/optimisation

In the same Vertex function, you compute things, you move the cursor and you draw dots. This seems like a lot for a single function. Also, you end up spending a lot of time performing the same computation. What you could do is define a function to compute coordinates, then call it for all vertices and use the results to draw dots first and draw edges later on.

def get_dot_coord(vertex):
    y = 0
    x = 0
    if vertex[0]:
        y = y - length
    for i in range(1,n):
        if vertex[i]:
            tmp_angle = (angle / ((n-1)/(n-i))) - math.pi/2
            y += length * math.sin(tmp_angle)
            x += length * math.cos(tmp_angle)
    return (x + offsetx, y + offsety)


vertices = list(itertools.product((True, False), repeat=n))
vertices_coords = {v: get_dot_coord(v) for v in vertices}

for i, v in enumerate(vertices_coords.values()):
    print("DRAWING VERTICES %d%% COMPLETE" % int((i/len(vertices))*100))
    t.goto(v)
    t.dot(3)

for x, y in itertools.combinations(vertices, 2):
    if sum(xi == yi for xi, yi in zip(x, y)) == n-1:
        t.goto(vertices_coords[x])
        t.pendown()
        t.goto(vertices_coords[y])
        t.penup()

Final code

The whole code now looks like:

import turtle
import time
import math
import math
import itertools

n = 3
length = 100
angle = 45
offsetx = 20
offsety = 30

t = turtle.Turtle()
t.penup()
t.shape('circle')
t.shapesize(0.01)
t.speed(0)

def get_dot_coord(vertex):
    y = 0
    x = 0
    if vertex[0]:
        y = y - length
    for i in range(1,n):
        if vertex[i]:
            tmp_angle = (angle / ((n-1)/(n-i))) - math.pi/2
            y += length * math.sin(tmp_angle)
            x += length * math.cos(tmp_angle)
    return (x + offsetx, y + offsety)


vertices = list(itertools.product((True, False), repeat=n))
vertices_coords = {v: get_dot_coord(v) for v in vertices}

for i, v in enumerate(vertices_coords.values()):
    print("DRAWING VERTICES %d%% COMPLETE" % int((i/len(vertices))*100))
    t.goto(v)
    t.dot(3)

for x, y in itertools.combinations(vertices, 2):
    if sum(xi == yi for xi, yi in zip(x, y)) == n-1:
        t.goto(vertices_coords[x])
        t.pendown()
        t.goto(vertices_coords[y])
        t.penup()

print("HYPERCUBE DRAWN")
nope = input("")

I have found new things to improve from my previous version of the code

Trigonometry

Adding an offset of math.pi / 2 before calling the sin/cos functions is not really needed as this can be done by swapping the functions (and their signs). You'd get:

        tmp_angle = angle / ((n-1)/(n-i))
        y -= length * math.cos(tmp_angle)
        x += length * math.sin(tmp_angle)

Simplifying operations

Instead of divising by a fraction, it may be easier to perform a multiplication and a division:

       tmp_angle = angle * (n-i) / (n-1)

Multiplication performed many times

When you look at the code in get_dot_coord, it is clear (and pretty logical) that you keep multiplying things by length. It may make sense to perform all operations in a world of length 1 and to multiply at the very end.

You'd get something like:

def get_dot_coord(vertex):
    x = 0
    y = -vertex[0]
    for i in range(1, n):
        if vertex[i]:
            tmp_angle = angle * (n-i) / (n-1)
            x += math.sin(tmp_angle)
            y -= math.cos(tmp_angle)
    return (x * length + offsetx, y * length + offsety)

I am still wondering why the first element of vertex needs to be handled in a special way...

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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