Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add itertools recipes to more-itertools #8

Closed
lonnen opened this issue Aug 3, 2012 · 12 comments
Closed

Add itertools recipes to more-itertools #8

lonnen opened this issue Aug 3, 2012 · 12 comments

Comments

@lonnen
Copy link
Contributor

lonnen commented Aug 3, 2012

http://docs.python.org/library/itertools.html#recipes has generically useful tools. Let's include them here.

@erikrose
Copy link
Collaborator

erikrose commented Aug 3, 2012

And let's have tests for them. Even just sanity-check, do-they-run-without-crashing tests. iterutils exists, but it has no tests.

@lonnen
Copy link
Contributor Author

lonnen commented Aug 7, 2012

Started hacking on this. Progress is here: https://github.com/Lonnen/more-itertools/compare/add-itertools-recipes

As functions get tests I add them to __all__. Not sure what I'm going to do when I get down to the random functions. Advice is appreciated, for that and on any of the work in progress. May not get back to finish this until next weekend.

@erikrose
Copy link
Collaborator

erikrose commented Aug 7, 2012

For the random functions, you can just call random.seed(...) first. Then they'll give known results.

@erikrose
Copy link
Collaborator

erikrose commented Aug 7, 2012

Or you could mock things into random.

@erikrose
Copy link
Collaborator

erikrose commented Aug 7, 2012

Looks great so far! I'm so glad you dove into this. A couple thoughts:

  • Let's stick the recipes in a recipes.py module next to __init__ and import the routines into __init__. This should make the lib a bit easier to explore for anybody poking at the code.
  • Let's pep-257-ify the docstrings while we're in there.

Tests look great—even more comprehensive than I envisioned.

@lonnen
Copy link
Contributor Author

lonnen commented Aug 9, 2012

What about mocking random? I've tried using seed, but the same seed produces different results in py2* and py3*

@erikrose
Copy link
Collaborator

erikrose commented Aug 9, 2012

Sure. Mock away.

@lonnen
Copy link
Contributor Author

lonnen commented Aug 9, 2012

After a little investigation, random.seed() is not a problem and setting the same seed will return the same values in python 2.* and python 3.*. The real issue was that random.choice() will not choose the same elements between different versions.

Thought about this for a while, and decided against mocking. Instead I avoid trying to test whether the results are truly random (and the associated philosophical rabbit hole), and instead wrote some tests to check that the results exhibit some expected properties. Some of these tests involve setting up and testing for highly likely events; the odds of which I've calculated and left in comments for now. I also tend to set the random seed in those examples, just to make sure things are not going to heisenfail.

@lonnen lonnen closed this as completed Aug 9, 2012
@lonnen lonnen reopened this Aug 9, 2012
@lonnen
Copy link
Contributor Author

lonnen commented Aug 9, 2012

Sorry for the accidental close. Errant mouse click.

The branch is updated, please take a look and let me know if you'd like more / less coverage somewhere, or if I went straight off the rails. I haven't addressed pep-257 and there are a few lines that aren't quite pep-8 compliant. I'll fix those next.

@erikrose
Copy link
Collaborator

erikrose commented Aug 9, 2012

Tests look great at first glance. I'd like to keep the recipes in a recipes.py, so I'd also like to do the tests in parallel, in a test_recipes.py. (We can make a tests folder.) And I'll want to rename Random_productTests to be CamelCasier. RandomProductTests is fine; people are smart enough to figure out what that means.

Really looking forward to merging this in. :-)

@lonnen
Copy link
Contributor Author

lonnen commented Aug 11, 2012

Ok. Check it out now. For the random docstring examples I use doctest:+SKIP, because of the problems getting random.choice to work across python versions.

@lonnen
Copy link
Contributor Author

lonnen commented Aug 12, 2012

#12

erikrose added a commit that referenced this issue Aug 16, 2012
* Make the new and old routines peers, and import them both into __init__.
* PEP 8
* Bump version to 2.1.
* Add recipes to documentation.
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

No branches or pull requests

2 participants