patternpythonModerate
N dimensional cubes
Viewed 0 times
dimensionalcubesstackoverflow
Problem
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) 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("")Solution
Generation of
You could use
with:
Import math
Imports are always put at the top of the file, just after any module
comments and docstrings, and before module globals and constants.
The
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
Thus, you could replace:
with
(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
Also, using this technique, you'd detect easily unused variable like
Rewriting angles computations
In :
you could:
And you'd get something like:
Your last iteration over
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
Once again, instead of keeping track of the number of equal pairs, you could use the
Correct data type
Instead of using literal strings
Separation of concern/optimisation
In the same
```
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
verticesYou 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 + 1with
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))*lengthyou could:
- use the
+=operator
- move the multiplication by
lengthto 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 againYour 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 wayOnce 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
Code Snippets
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)import itertools
vertices = list(itertools.product('10', repeat=n))r = 0
for i in vertices:
print("DRAWING VERTICES,",int((r/(2**n))*100),"% COMPLETE")
Vertex(i)
r = r + 1for i, v in enumerate(vertices):
print("DRAWING VERTICES,",int((i/(2**n))*100),"% COMPLETE")
Vertex(v)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()Context
StackExchange Code Review Q#149095, answer score: 10
Revisions (0)
No revisions yet.