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

Added print_bin() method #32

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

R3tr0BoiDX
Copy link

I added a method, that prints outs all the bins. May need an additional option to make this optional.

@benmaier
Copy link
Owner

Hi, thanks for the contribution! I don't have time to check this properly atm, but the following things need to be done before merging is possible

  1. please do not use format strings (f"{}"), they were introduced in Python 3.6 and will break backwards-compatibility
  2. add a boolean keyword argument to the respective functions that switches printing bins on and off, please have it off as default (because people use this on large data sets, which will make the CLI unusable)
  3. add a similar boolean option to the CLI procedure

@R3tr0BoiDX
Copy link
Author

Okay, I added the -p option. I never worked with the OptionParser. I hope I did that in the right manner.

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.

2 participants