Time to have a look at refactoring all the duplicated transform code in the main module. I expect I will start slow and iterate. Moving a few lines at a time into one or more functions. Making adjustments in other modules as needed.
Refactor Transform Code
Generating Transform Paramaters (Iteration 1)
In the transform routes there appears to be a bit of code called in every case. That code sets up some basic values to use in generating the transforms: angles for the rotational transforms, translation distance for the linear transforms, etc. That code looks like the following:
# get angles and such
lld_rot, lld_deg, trdy, y_mlt = sal.get_transform_base(t_xs, t_ys)
# get rand quadrant order
do_qd = list(range(g.nbr_tr))
do_qd2 = sal.get_rnd_qds(do_qd)
Bad choice of words: quadrant order. There are not always exactly 4 transforms. But I started development with only four and quadrant at the time was certainly the correct word.
The exception to the above code is the mosaic “transform to transform” spirograph image type. In that case we get two lists of transform order. And try to remove duplicate values for each index. We don’t really want to plot between the same transform— doesn’t accomplish much.
# get rand quadrant orders
do_qd = sal.get_rnd_qds(list(range(g.nbr_tr)))
do_qd2 = sal.get_rnd_qds(do_qd)
if not g.drp_tr:
chk_fn = sal.lists_overlap
else:
chk_fn = sal.lists_equal
qlcnt = 0
while chk_fn(do_qd, do_qd2) and qlcnt < 5:
do_qd2 = sal.get_rnd_qds(do_qd)
qlcnt += 1
But an if block should handle that variation.
So, let’s create a new function, get_trfm_params(). Expect it should be get_transform_parameters(), but I’m not big on full words making for big function names. We will need to pass it at least one parameter, the image type for the if block to function correctly. And, as I look at things we will likely need to pass the spirograph curve data. We may also need to return different bits of info depending on the spirograph image type—though at the moment I think we can avoid doing so.
ef get_trfm_params(sx, sy, i_type):
# get angles and such
lld_rot, lld_deg, trdy, y_mlt = sal.get_transform_base(sx, sy)
# get rand order for the number of transforms
if i_type != 'mosaic_t2t':
do_qd = list(range(g.nbr_tr))
do_qd2 = sal.get_rnd_qds(do_qd)
else:
do_qd = sal.get_rnd_qds(list(range(g.nbr_tr)))
do_qd2 = sal.get_rnd_qds(do_qd)
# which check function to use
if not g.drp_tr:
chk_fn = sal.lists_overlap
else:
chk_fn = sal.lists_equal
# if check function unhappy, generate new random order and repeat, but no more than 5 times
qlcnt = 0
while chk_fn(do_qd, do_qd2) and qlcnt < 5:
do_qd2 = sal.get_rnd_qds(do_qd)
qlcnt += 1
return lld_rot, lld_deg, trdy, y_mlt, do_qd, do_qd2
Okay, let’s refactor all the affected routes. I will only show the changes (git diff) for a couple of them.
PS R:\learn\py_play\sp_fa> git diff main.py
diff --git a/main.py b/main.py
index 462d9b8..5445930 100644
--- a/main.py
+++ b/main.py
@@ -203,12 +228,7 @@ def sp_basic_t():
t_xs, t_ys, *_ = get_curve_data('basic_t', request.form, g.t_pts)
setup_image(ax)
- # get angles and such
- lld_rot, lld_deg, trdy, y_mlt = sal.get_transform_base(t_xs, t_ys)
-
- # get rand quadrant order
- do_qd = list(range(g.nbr_tr))
- do_qd2 = sal.get_rnd_qds(do_qd)
+ lld_rot, lld_deg, trdy, y_mlt, do_qd, do_qd2 = get_trfm_params(t_xs, t_ys, 'basic_t')
# Generate transformed datasets
for i in do_qd2:
@@ -603,20 +623,7 @@ def sp_mosaic_t2t():
# print(f"POST[shape]: {request.form.get('shape')}, POST[shp_mlt]: {request.form.get('shp_mlt')}")
r_xs, r_ys, m_xs, m_ys, m2_xs, m2_ys = get_curve_data('mosaic', request.form, g.t_pts)
- # get angles and such
- lld_rot, lld_deg, trdy, y_mlt = sal.get_transform_base(r_xs, r_ys)
-
- # get rand quadrant orders
- do_qd = sal.get_rnd_qds(list(range(g.nbr_tr)))
- do_qd2 = sal.get_rnd_qds(do_qd)
- if not g.drp_tr:
- chk_fn = sal.lists_overlap
- else:
- chk_fn = sal.lists_equal
- qlcnt = 0
- while chk_fn(do_qd, do_qd2) and qlcnt < 5:
- do_qd2 = sal.get_rnd_qds(do_qd)
- qlcnt += 1
+ lld_rot, lld_deg, trdy, y_mlt, do_qd, do_qd2 = get_trfm_params(r_xs, r_ys, 'mosaic_t2t')
# drop first data row?
if g.n_whl == 3:
And the five routes appear to work as expected. (You’ll just have to take my word for it.)
Plotting Transforms (Iteration 2)
The only thing really left is the actual production and plotting of each transform. I expect most of them will be fairly similar (based on memory, yet to actually confirm). Again likely with the exception of the mosaic “transform to transform” spirograph image type/route. Figure it is worth a try.
Here’s the code for the routes of interest.
# basic_t
# Generate transformed datasets and plot
for i in do_qd2:
dstx, dsty = sal.get_transform_data(t_xs, t_ys, lld_rot[i], lld_rot[i], trdy)
ax.plot(dstx[-1], dsty[-1], lw=g.ln_w, alpha=1)
ax.autoscale()
# clw_btw_t
tmp_rot = 0
# Generate transformed datasets
for i in do_qd2:
if lld_deg[i] in [0.0, 90.0, 180.0, 270.0]:
tmp_rot = 1.2 * lld_rot[i]
dstx1, dsty1 = sal.f(t_xs[-1], clwy_1, tmp_rot, lld_rot[i], trdy)
dstx2, dsty2 = sal.get_transform_data(t_xs[-1], clwy_2, tmp_rot, lld_rot[i], trdy)
else:
dstx1, dsty1 = sal.get_transform_data(t_xs[-1], clwy_1, lld_rot[i], lld_rot[i], trdy)
dstx2, dsty2 = sal.get_transform_data(t_xs[-1], clwy_2, lld_rot[i], lld_rot[i], trdy)
pp_clr = sal.clw_btw_plot(ax, dstx1, dsty1, dsty2, dppc=g.dpp_clr)
# !! and there you go, a variation already
# code for gnarly_t looks a lot like basic_t
# mosaic_t
# drop first data row?
if g.n_whl == 3:
src_st = 0
else:
src_st = 1
src = np.array([r_xs[src_st:], r_ys[src_st:]], dtype="float")
# Generate transformed datasets
for i in do_qd2:
dstx, dsty = sal.get_transform_data(src[0], src[1], lld_rot[i], lld_rot[i], trdy)
sal.btw_n_apart(ax, dstx, dsty, dx=g.bw_dx, ol=g.bw_o, r=g.bw_r, fix=None, mlt=g.bw_m, sect=g.bw_s)
ax.autoscale()
sal.cnt_btw += 1
# mosaic_t2t
# drop first data row?
if g.n_whl == 3:
src_st = 0
else:
src_st = 1
src = np.array([r_xs[src_st:], r_ys[src_st:]], dtype="float")
setup_image(ax)
# Generate transformed datasets
for i in range(g.nbr_tr):
if do_qd[i] == do_qd2[i]:
if g.DEBUG:
print(f"\tDEBUG: dropping transform {do_qd[i]} == {do_qd2[i]}")
continue
dstx1, dsty1 = sal.get_transform_data(src[0], src[1], lld_rot[do_qd[i]], lld_rot[do_qd[i]], trdy)
_, dsty2 = sal.get_transform_data(src[0], src[1], lld_rot[do_qd2[i]], lld_rot[do_qd2[i]], trdy)
rys2i = sal.btw_qds(ax, dstx1, dsty1, dsty2, mlt=g.bw_m, sect=g.bw_s)
ax.autoscale()
sal.cnt_btw += 1
Definitely some considerable variation. But, you know I do believe we can do some code reduction. I see two approaches. Do all the transform generation and plotting in a single function. Certainly doable. Or do all the transform generation in a single function passing back to the route all the datasets to be plotted and let it do that bit of the work. Guess it comes down to a preference for or against side affects and the potential impact on system memory of a list of unknown size with multiple lists of possibly considerable size.
Personally, I have never had a concern about side affects. Especially in my own code. That feeling would likely be different if I was writing a module to be offered for general use. But, that’s not the case here. And, I’d like to test the memory useage approach to see how that goes. If I don’t like the performance hit, I will go back to the side affect approach.
Also to save memory, I am for each given image type going to return the absolute minimum data required to generate the spirograph image. For example for the basic spirograph it will be two rows of plotting points (one each for x and y data) for each transform (i.e. the value of g.nbr_tr).
I started by only coding what was needed to return the necessary data for the basic spirograph with transforms.
def make_transforms(d_xs, d_ys, i_type, do_t2, lld_rot, trdy, y_mlt=None, do_t1=[]):
dt_x, dt_y = [], []
if i_type == 'basic_t':
for i in do_t2:
dstx, dsty = sal.get_transform_data(d_xs, d_ys, lld_rot[i], lld_rot[i], trdy)
dt_x.append(dstx[-1])
dt_y.append(dsty[-1])
return dt_x, dt_y
Refactored the route and tested. And the testing worked just fine.
PS R:\learn\py_play\sp_fa> git diff main.py
diff --git a/main.py b/main.py
index 8fa0d0b..c8f692b 100644
--- a/main.py
+++ b/main.py
@@@ -230,11 +246,10 @@ def sp_basic_t():
lld_rot, lld_deg, trdy, y_mlt, do_qd, do_qd2 = get_trfm_params(t_xs, t_ys, 'basic_t')
# Generate transformed datasets
- for i in do_qd2:
- dstx, dsty = sal.get_transform_data(t_xs, t_ys, lld_rot[i], lld_rot[i], trdy)
+ dt_x, dt_y = make_transforms(t_xs, t_ys, "basic_t", do_qd2, lld_rot, trdy, y_mlt=y_mlt, do_t1=do_qd)
- ax.plot(dstx[-1], dsty[-1], lw=g.ln_w, alpha=1)
+ for i in range(len(dt_x)):
+ ax.plot(dt_x[i], dt_y[i], lw=g.ln_w, alpha=1)
ax.autoscale()
data = fini_image(fig, ax)
Next I refactored the new function to deal with the gnarly spirograph images with transforms. A minor change for the data to be returned. I left the decision on the removal of rows from the dataset in the route.
def make_transforms(d_xs, d_ys, i_type, do_t2, lld_rot, trdy, y_mlt=None, do_t1=[]):
dt_x, dt_y = [], []
if i_type in ['basic_t', 'gnarly_t']:
for i in do_t2:
dstx, dsty = sal.get_transform_data(d_xs, d_ys, lld_rot[i], lld_rot[i], trdy)
if i_type == 'basic_t':
dt_x.append(dstx[-1])
dt_y.append(dsty[-1])
else:
dt_x.append(dstx)
dt_y.append(dsty)
return dt_x, dt_y
Refactor the route and test. And, again the tests were positive.
PS R:\learn\py_play\sp_fa> git diff main.py
diff --git a/main.py b/main.py
index 8fa0d0b..0f6636e 100644
--- a/main.py
+++ b/main.py
@@ -494,10 +507,10 @@ def sp_gnarly_t():
lld_rot, lld_deg, trdy, y_mlt, do_qd, do_qd2 = get_trfm_params(r_xs, r_ys, 'gnarly_t')
# Generate transformed datasets
- for i in do_qd2:
- dstx, dsty = sal.get_transform_data(r_xs, r_ys, lld_rot[i], lld_rot[i], trdy)
+ dt_x, dt_y = make_transforms(r_xs, r_ys, "gnarly_t", do_qd2, lld_rot, trdy, y_mlt=y_mlt, do_t1=do_qd)
- ax.plot(dstx, dsty, lw=g.ln_w, alpha=g.alph, zorder=g.pz_ord)
+ for i in range(len(dt_x)):
+ ax.plot(dt_x[i], dt_y[i], lw=g.ln_w, alpha=1)
ax.autoscale()
data = fini_image(fig, ax)
Thought I should include at least one image in the post.
Let’s add the mosaic spirograph images with transforms. The transform to transform variant requires an extra array/list. So, I will be modifying the function to return 3 arrays. The last will be empty for all but the mosaic_t2t image type. Which means a slight modifcation to the variable list for the function calls above. For the basic_t image, it will now be:
dt_x, dt_y, _ = make_transforms(t_xs, t_ys, "basic_t", do_qd2, lld_rot, trdy, y_mlt=y_mlt, do_t1=do_qd)
The function now looks like the following. Pretty much just copying previous code. A few new if blocks needed.
def make_transforms(d_xs, d_ys, i_type, do_t2, lld_rot, trdy, y_mlt=None, do_t1=[]):
dt_x, dt_y, dt_y2 = [], [], []
if i_type in ['basic_t', 'gnarly_t']:
for i in do_t2:
dstx, dsty = sal.get_transform_data(d_xs, d_ys, lld_rot[i], lld_rot[i], trdy)
if i_type == 'basic_t':
dt_x.append(dstx[-1])
dt_y.append(dsty[-1])
else:
dt_x.append(dstx)
dt_y.append(dsty)
if 'mosaic' in i_type:
# drop first data row?
if g.n_whl == 3:
src_st = 0
else:
src_st = 1
src = np.array([d_xs[src_st:], d_ys[src_st:]], dtype="float")
if i_type == 'mosaic_t':
for i in do_t2:
dstx, dsty = sal.get_transform_data(src[0], src[1], lld_rot[i], lld_rot[i], trdy)
dt_x.append(dstx)
dt_y.append(dsty)
elif i_type == 'mosaic_t2t' and do_t1:
for i in range(g.nbr_tr):
if do_t1[i] == do_t2[i]:
continue
dstx1, dsty1 = sal.get_transform_data(src[0], src[1], lld_rot[do_t1[i]], lld_rot[do_t1[i]], trdy)
_, dsty2 = sal.get_transform_data(src[0], src[1], lld_rot[do_t2[i]], lld_rot[do_t2[i]], trdy)
dt_x.append(dstx)
dt_y.append(dsty1)
dt_y2.append(dsty2)
return dt_x, dt_y, dt_y2
We’re using the throwaway variable (underscore) to ignore that third value. Only the one route (mosaic_t2t) will actually use it. Note, that’s my name, not sure it has a specific name in Python circles.
Okay, let’s start with the mosaic_t route.
@@ -572,27 +594,11 @@ def sp_mosaic_t():
lld_rot, lld_deg, trdy, y_mlt, do_qd, do_qd2 = get_trfm_params(r_xs, r_ys, 'mosaic_t')
- # drop first data row?
- if g.n_whl == 3:
- src_st = 0
- else:
- src_st = 1
- src = np.array([r_xs[src_st:], r_ys[src_st:]], dtype="float")
+ d_xs, d_ys, _ = make_transforms(r_xs, r_ys, 'mosaic_t', do_qd2, lld_rot, trdy, y_mlt=y_mlt)
# Generate transformed datasets
for i in do_qd2:
- dstx, dsty = sal.get_transform_data(src[0], src[1], lld_rot[i], lld_rot[i], trdy)
-
- sal.btw_n_apart(ax, dstx, dsty, dx=g.bw_dx, ol=g.bw_o, r=g.bw_r, fix=None, mlt=g.bw_m, sect=g.bw_s)
+ sal.btw_n_apart(ax, d_xs[i], d_ys[i], dx=g.bw_dx, ol=g.bw_o, r=g.bw_r, fix=None, mlt=g.bw_m, sect=g.bw_s)
ax.autoscale()
sal.cnt_btw += 1
And that seems to work just fine. Now the mosaic_t2t route.
@@ -623,32 +629,19 @@ def sp_mosaic_t2t():
lld_rot, lld_deg, trdy, y_mlt, do_qd, do_qd2 = get_trfm_params(r_xs, r_ys, 'mosaic_t2t')
- # drop first data row?
- if g.n_whl == 3:
- src_st = 0
- else:
- src_st = 1
- src = np.array([r_xs[src_st:], r_ys[src_st:]], dtype="float")
- if g.DEBUG:
- print(f"len(src[0]): {len(src[0])} -> {len(src[0][0])}, len(src[1]): {len(src[1])} -> {len(src[1][0])}")
-
setup_image(ax)
# only change multipliers the first time, not on later iterations
# following being set in function setup_image(ax), so not needed here
# sal.cnt_btw = 0
+ d_xs, d_ys, d_ys2 = make_transforms(r_xs, r_ys, 'mosaic_t2t', do_qd2, lld_rot, trdy, y_mlt=y_mlt, do_t1=do_qd)
+
# Generate transformed datasets
for i in range(g.nbr_tr):
if do_qd[i] == do_qd2[i]:
continue
-
- dstx1, dsty1 = sal.get_transform_data(src[0], src[1], lld_rot[do_qd[i]], lld_rot[do_qd[i]], trdy)
- _, dsty2 = sal.get_transform_data(src[0], src[1], lld_rot[do_qd2[i]], lld_rot[do_qd2[i]], trdy)
-
- rys2i = sal.btw_qds(ax, dstx1, dsty1, dsty2, mlt=g.bw_m, sect=g.bw_s)
+ rys2i = sal.btw_qds(ax, d_xs[i], d_ys[i], d_ys2[i], mlt=g.bw_m, sect=g.bw_s)
ax.autoscale()
sal.cnt_btw += 1
And it is also working as expected. Once I remembered to change ‘mosaic_t’ to ‘mosaic_t2t’ after copying the line from the first to the second route.
Done
This is getting to be a rather long post. And I still have the clw_btw_t route to look at. But, that I think will have to wait for another day.
Until then, remember that it’s almost always a good time to refactor your code.