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

Please make consumable as a library #3

Closed
DidgetyTech opened this issue Nov 1, 2018 · 7 comments
Closed

Please make consumable as a library #3

DidgetyTech opened this issue Nov 1, 2018 · 7 comments

Comments

@DidgetyTech
Copy link
Contributor

DidgetyTech commented Nov 1, 2018

This is precisely the algorithm I'm looking for to use in my production system! But to use it, there's a few things to change to make it useful as a library:

  1. Return the sample result in sample_n(). It implicitly returns None now.
  2. Use logging and single line printouts instead of print.
    • With logging, the consuming code turn on or off logging or set the levels to log. With single lines, logs are easily greppable. I think the printout is fine for a script, but it's not neighborly as a library.

If this works for you, I can submit a pull request tomorrow.

@asmith26
Copy link
Owner

asmith26 commented Nov 2, 2018

Hi @hoganbc,

Thanks for your great suggestions. I particularly think getting sample_n() to return something instead of just None is a great idea. Pull requests very welcome!

Thanks again

@DidgetyTech
Copy link
Contributor Author

PR #4

@asmith26
Copy link
Owner

asmith26 commented Nov 3, 2018

Thanks very much @hoganbc - a great and very clear pull request!

Updated PyPi accordingly.

@asmith26 asmith26 closed this as completed Nov 3, 2018
@DidgetyTech
Copy link
Contributor Author

Thanks!

@DidgetyTech
Copy link
Contributor Author

Do you mind adding a tag to track the versions? This makes it really nice to know which commits were released with which version.

@asmith26
Copy link
Owner

asmith26 commented Nov 5, 2018

Happily will - I haven't really used git tags before, what command would you recommend?

@DidgetyTech
Copy link
Contributor Author

Looks like it's tagged now! The tag subcommand should be pretty straightforward, similar to using the branch subcommand.

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