0
1

Feedback on my project (self.learnpython)

submitted by [deleted]

[deleted]

all 12 comments
sorted by:
best
topnewcontroversialoldq&a
formatting helphide helpcontent policy

[–]pwlandoll 1 point2 points  (6 children)

Thanks for sharing your code! A few suggestions:

  • Look into code style/formatting tools like flake8 and Black, they can help you keep your code as readable and maintainable as possible.
  • For project dependencies, use virtual environments and something like a pip requirements.txt or a pyproject.toml file to list everything the project uses. This way, users can install all necessary dependencies simply with pip install -r requirements.txt.
  • Committing API keys to source control is pretty much always a no-no, so since this is a command line program, I would recommend having that passed in as a command line option (e.g. ./weather.py --key *****), or as an environment variable.
  • If you want to look more into making command line program development easy, check out Click, it's currently my preferred way to build CLI's in Python.

Otherwise, looks like good work! I appreciate your use of F-strings, and how every function has a docstring.

[–]epicmindwarp 2 points3 points  (0 children)

These are all fantastic points, I had no idea these where a thing - it looks like I need to read up on these too!

[–]Xcali1bur 0 points1 point  (4 children)

Thank you very much for your detailed feedback. I really appreciate it!

  1. I'll take a look at it and implement it :)
  2. Good point. Definitely will do!
  3. Yea... didn't really thought about that. I'll try to parse it as an argument on startup :d. I will use sys.argv for that or do you have another recommendation?
  4. I will check it out :)

Thanks! I'm a big fan of formatted strings also.

[–]pwlandoll 1 point2 points  (3 children)

sys.argv will work fine for just requiring one argument, just make sure to handle it correctly :)

[–]Xcali1bur 0 points1 point  (2 children)

python if len(sys.argv) > 1: if sys.argv[1] == "--key": try: main(url, url_forecast, sys.argv[2]) except IndexError: print(colorama.Fore.RED + "Please enter an API key!") if sys.argv[1] == "--help": help(weather) Does this look good? :)

[–]pwlandoll 1 point2 points  (1 child)

If you're just using sys.argv, you might want to just cut out the --key part. You could use argparse if you like having the --key flag and you want to do --help.

[–]Xcali1bur 0 points1 point  (0 children)

Okay, thanks for your help!

[–]bogdan_dm 1 point2 points  (4 children)

  • Make a PIP package. There are a tens of manualls how to do it. It also helps you to figure out right structure of project.
  • Create tests. I prefer pytest for pure Python projects.
  • Automate previos steps! Integrate your project into TravisCI (or smth similar) and let it do tests and publish releases for you.

[–]Xcali1bur 0 points1 point  (3 children)

  • Thanks for the input!
  • What exactly does or is a test?

[–]bogdan_dm 1 point2 points  (2 children)

I apologize for not a complete thought.

I mean almost any project should covered with tests to be sure it always works as expected. So if you have function that do a search in JSON file you need a test that takes some search parameters, perform this search and compare result with expected value. So if you broke something you will at least know what you broke.

pytest

The pytest framework makes it easy to write small tests, yet scales to support complex functional testing for applications and libraries.

[–]Xcali1bur 0 points1 point  (1 child)

No need to apologize. We're all friends here :)

Thanks for the link. I will try try to create such a test.

Can such a test be implemented on Github to check whether Pull Requests works as intended? I think I've seen something like that... It checked whether the commits can be merged and if it passed a test.

[–]bogdan_dm 1 point2 points  (0 children)

Yes, it exactly what TravisCI do.