Skip to content

Conversation

@stanley6716
Copy link

No description provided.

Copy link
Contributor

@StephLin StephLin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many thanks for your contribution!

The code is almost correct, while there are some programming issues. Please fix them accordingly.

We format our code with Black and lint them with Flake8 in this project. Please checkout here (offline code linting) and here (coding style) for details. Thank you!

@@ -1,0 +1,36 @@
import pylab
import numpy as np
from random import *
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prevent using from ... import *.

Suggested change
from random import *
from random import random

map=[[0x8,0x8,0x7f,0x49,0x7f,0x8,0x8],[0x8,0x8,0x3e,0x2a,0x7f,0x14,0x63]]
R,C=7,7
ds=5
#m=int(input("> "))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove unused code.

a=2*pi*random()
sina=np.sin(a)
cosa=np.cos(a)
ra=(1/m)*(0.8+0.2*random())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ra=(1/m)*(0.8+0.2*random())
ra=(1/m)*(2.0+0.2*random())

The radius $0.8 + X \sim \mathcal{U}_{[0, 0.2)}$ seems to be too small 🤔:

image

I think the radius should be enlarged. Like $2.0 + X \sim \mathcal{U}_{[0, 0.2)}$:

image

#m=int(input("> "))
m = 1
ds=20
pylab.axis("off")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest adding pylab.axis("equal") for isometric scaling.

Suggested change
pylab.axis("off")
pylab.axis("off")
pylab.axis("equal")

@StephLin StephLin mentioned this pull request Jan 14, 2023
4 tasks
@StephLin StephLin linked an issue Jan 14, 2023 that may be closed by this pull request
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unfinished questions

2 participants