3
\$\begingroup\$

When tossing a dice many many time all numbers tend to appear the same number of times, but if the number of throws is small then some numbers may appear more or less often than others, even considerably if the sample size is small enough.

This program shows a updating in real time graph of SIDES bars (6 in default), each presenting how many times a number has appeared so far.

Please ignore that the numbers on the X axis are wrong, that is a minor bug-fix for later, all of the visualization is there.

A screenshot

My code feels a bit messy, please help in organization and simplification:

import random

import numpy as np
import matplotlib.pyplot as plt
import matplotlib.animation as animation

SIDES = 6
HEIGHT = 100

def update_results(results):
    die_toss = random.randint(0, SIDES - 1)
    return [x + (1 if i == die_toss else 0) for i, x in enumerate(results)]

def plot_points(ps):
    plt.scatter(*zip(*ps))

def animate(frameno):
    global x
    x = update_results(x)
    n, _ = np.histogram(x, SIDES)
    n = x
    for rect, h in zip(patches, n):
        rect.set_height(h)
    print(x, max(x) / float(min(x)))
    return patches

if __name__ == "__main__":
    x = [ 1 for _ in range(SIDES) ]

    fig, ax = plt.subplots()
    n, bins, patches = plt.hist(x, SIDES, normed=1, facecolor='green', alpha=0.75)

    frames = 500
    ani = animation.FuncAnimation(fig, animate)

    axes = plt.gca()
    axes.set_ylim([0, HEIGHT])

    plt.show()

    plt.show()

    for i in range(100):
        plt.bar(range(SIDES), x)
        x =update_results(x)
        print(x)
        plt.show()
        plt.clf()
\$\endgroup\$
4
\$\begingroup\$

You are doing a lot of unneeded stuff. I completely deleted the for loop in the main part as well as the second plt.show. I also removed the plot_points function. None of this changed the functionality of the program, as far as I can tell.

matplotlib.animation.FuncAnimation takes an optional fargs argument which contains arguments to be passed on to the animate function. So this gets rid of needing global variables.

I made update_results modify x. This makes the update_results function obsolete, because now we only need to increment the element at the index chosen by the dice, which we can inline in the animate function.

I made the starting array start with all zeros, which is more realistic. Because of this I had to include a min(x) or 1 in animate to ensure that if the minimum is 0, 1 is used. I used the fact that [0] * 3 == [0, 0, 0] to simplify the initialization.

I also made the y limit fix you suggested by updating it in the animate function and made it obvious that the first parameter of the animate function is unused by using _ for it.

import random

import numpy as np
import matplotlib.pyplot as plt
import matplotlib.animation as animation

SIDES = 6


def animate(_, x, patches):
    die_toss = random.randrange(SIDES)
    x[die_toss] += 1
    for rect, h in zip(patches, x):
        rect.set_height(h)
    print(x, max(x) / float(min(x) or 1))
    axes = plt.gca()
    axes.set_ylim([0, max(x) + 1])
    return patches


def main():
    x = [0] * SIDES

    fig = plt.figure()
    n, bins, patches = plt.hist(
        x, SIDES, facecolor='green', alpha=0.75)

    frames = 500
    ani = animation.FuncAnimation(fig, animate, fargs=(x, patches))
    plt.show()

if __name__ == "__main__":
    main()
\$\endgroup\$
2
\$\begingroup\$

The code is generally not understandable for me, who is unfamiliar with the code, but already familiar with what it should be doing. This is because of multiple reasons:

  • a lot of "uncategorized" and undocumented things under the if __name__ == "__main__":
  • functions don't have docstrings and there are no helpful comments (they are not always needed, but I think they would help in some parts of this code)
  • global variables are used - this requires me to jump from different parts of the code to other to see where a global variable is used and how is it set
  • there are some magic numbers used inside frames = 500 and range(100) inside the code itself - they should probably be moved to the top and assigned meaningful names
  • animate() function accepts a single argument, but it is not used inside the function
  • plt.show() is called multiple times - is this intentional?

And, also, there are some PEP8 violations:

  • two newlines between the function definitions
  • no spaces after the [ and before the ]
  • space around the =

Also, you can extract all the plot configuration and preparation steps into a, for example, plot_setup function (this is called "Extract Method" refactoring method).

As a side note, you may define the range(SIDES) before the loop to avoid reinitializing the range on every iteration of the loop.

\$\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.