llimllib / pymag-trees

Code from the article "Drawing Good-looking Trees" in Python Magazine
http://billmill.org/pymag-trees/
Do What The F*ck You Want To Public License
187 stars 33 forks source link

Possible small code errors in listings 5 and 11? #12

Open masonhieb opened 2 months ago

masonhieb commented 2 months ago

On this line here in listing 11, shouldn't tree.y = depth be at the top of the setup function?

I was implementing this recently and, with the function written as shown, I believe that code would only set the y values for trees with no children, but it should be setting the y values for every tree, should it not? Or is there an assumption here that the y-values are already set elsewhere (since most of the real complexity comes from the x coordinates, not the y coordinates)?

Additionally, in listing 5 this tree.offset class variable is used but seems to be accidentally copied over from listing 4. Should that value instead be tree.mod?

llimllib commented 2 months ago

I don't know, unfortunately! This code was written 17 years ago and the drawings were done on a piece of software that no longer exists.

I would love to apply corrections, but I'm not sure that I know how to verify them any longer; I'm open to your opinions on how best to proceed.

llimllib commented 2 months ago

Okay, I've started a typescript translation.

Definitely in listing 5 either line 44 should be tree.mod or line 40 should be tree.offset, will deal with listing 11 when I get there

masonhieb commented 2 months ago

Ok, thanks for taking a look! I implemented listing 11 in C++ and only the childless nodes of my tree were at the correct y-coordinate. I hope you'll still keep the original up somewhere if your plan is to create a more modern version.

llimllib commented 2 months ago

definitely. Thanks for filing this issue!