Skip to content

Conversation

@AVerzier
Copy link

@AVerzier AVerzier commented Sep 12, 2019

Uses numpy v1.17+ random array generator
And python 3.6+ f-strings ...

100 iterations

@AustinTSchaffer
Copy link

AustinTSchaffer commented Sep 12, 2019

This adds the project's first 3rd party library, but does not add a setup.py, nor does it add a requirements.txt. Also, it forces the original author to upgrade their default python interpreter to Python 3.6+ when the original code is written in 2.7, probably due to familiarity and the fact that it was just "pub code". I agree that all new Python code should be Python 3 compliant, but this seems a little too presumptuous.

@achampion
Copy link

achampion commented Sep 13, 2019

If you are going to use a 3rd party library you might as well use pandas:

In [1]:
from __future__ import print_function
from __future__ import division

import pandas as pd
import random

def cross_river(dist):
    leaps = 0
    while dist > 0:
        dist -= random.randrange(dist)+1
        leaps += 1
    return leaps

def frog_simulation(final_dist=100, max=6):
    trials = 10**max
    results = {}
    for dist in range(1, final_dist+1):
        result[dist] = sum(cross_river(dist) for _ in range(trials)) / trials

    return results

s = pd.Series(frog_simulation(10, 6))
s.plot()
print(s)

Out[1]:
1     1.000000
2     1.499309
3     1.833457
4     2.083143
5     2.282706
6     2.451438
7     2.593378
8     2.717028
9     2.827266
10    2.929268
dtype: float64

image

@AVerzier
Copy link
Author

You're both right
That's the first time I ever submit a pull request, and I didn't think about all of this ...
I just wanted to give my solution to the problem

I didn't know pandas library, but the code looks cleary nicer :)

@AustinTSchaffer
Copy link

Don't sweat it, that's why PRs have an associated discussion thread.

@pcholt
Copy link

pcholt commented Sep 13, 2019

PRs in commercial software can be utterly brutal - this is all very mild.

I'm happy to add some output command line parameters, for csv output formatting to import into a spreadsheet for graphing

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.

4 participants