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

Code cleanup, string formatting, version importing #3

Merged
merged 2 commits into from Oct 2, 2020

Conversation

byarmis
Copy link

@byarmis byarmis commented Oct 1, 2020

  • Made it easier to bump the version-- now, just have to update the version in one place instead of two
  • Changed the string formatting so people don't need to remember \ before ' in some strings
  • Changed constant names to be more descriptive
  • main() function now returns a string instead of directly printing, allowing other tools / functions to import and run the program without printing unless they want to
  • Added backwards-compatible type-hinting

@diptangsu
Copy link
Owner

That is a wonderful amount of refactoring! Thanks!
I only wrote this to learn how a python package can be published to PyPi, never really thought someone else would even touch this code LOL!

However, this particular change does not look right to me. I haven't really checked it, I'll do it once I get some time.
image

Since this is a python module, we need to run it using the -m flag. Python's import system is pretty weird even when you properly understand it actually.

If you think it's not a mistake on your part, just let me know and I'll merge it, otherwise just make that one change and I'll merge it!

@diptangsu diptangsu merged commit c6cd071 into diptangsu:master Oct 2, 2020
@diptangsu
Copy link
Owner

image
It is wrong. I will make the changes and fix it.

If it's a module, we can't run it as a python script; we need to run it with the -m flag, in which case it breaks.

@byarmis
Copy link
Author

byarmis commented Oct 2, 2020

I apologize for that, I didn't realize that the proper way to run it was with python -m, not just python

@diptangsu
Copy link
Owner

No problem brother! It's two people now who learnt something new from this project! haha!
I didn't know this either until I wrote this code!

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.

None yet

2 participants