this post was submitted on 25 Apr 2025
23 points (89.7% liked)
Python
7061 readers
57 users here now
Welcome to the Python community on the programming.dev Lemmy instance!
๐ Events
Past
November 2023
- PyCon Ireland 2023, 11-12th
- PyData Tel Aviv 2023 14th
October 2023
- PyConES Canarias 2023, 6-8th
- DjangoCon US 2023, 16-20th (!django ๐ฌ)
July 2023
- PyDelhi Meetup, 2nd
- PyCon Israel, 4-5th
- DFW Pythoneers, 6th
- Django Girls Abraka, 6-7th
- SciPy 2023 10-16th, Austin
- IndyPy, 11th
- Leipzig Python User Group, 11th
- Austin Python, 12th
- EuroPython 2023, 17-23rd
- Austin Python: Evening of Coding, 18th
- PyHEP.dev 2023 - "Python in HEP" Developer's Workshop, 25th
August 2023
- PyLadies Dublin, 15th
- EuroSciPy 2023, 14-18th
September 2023
- PyData Amsterdam, 14-16th
- PyCon UK, 22nd - 25th
๐ Python project:
- Python
- Documentation
- News & Blog
- Python Planet blog aggregator
๐ Python Community:
- #python IRC for general questions
- #python-dev IRC for CPython developers
- PySlackers Slack channel
- Python Discord server
- Python Weekly newsletters
- Mailing lists
- Forum
โจ Python Ecosystem:
๐ Fediverse
Communities
- #python on Mastodon
- c/django on programming.dev
- c/pythorhead on lemmy.dbzer0.com
Projects
- Pythรถrhead: a Python library for interacting with Lemmy
- Plemmy: a Python package for accessing the Lemmy API
- pylemmy pylemmy enables simple access to Lemmy's API with Python
- mastodon.py, a Python wrapper for the Mastodon API
Feeds
founded 2 years ago
MODERATORS
you are viewing a single comment's thread
view the rest of the comments
view the rest of the comments
I actually really like doing code reviews, so I'm happy to dig into this one. I have a lot of comments, but please don't take this as a criticism of your skills, or even of the code itself. I'm just... really thorough ;-) Hopefully, you'll find some of this helpful.
Formatting
The first suggestion I have is to install a code formatter like ruff or black. The Python community develops our code to a standard called pep8 which you're violating all over the place here, making it harder to read your code. Rather than having to go through it all and manually conforming it though, tools like
ruff
orblack
will do it with one command. A lot of developers will even plug them into their IDE to run against their code on-save, so you don't even have to think about running it.CONSTANTS vs attributes
Pep8 says that constants should be in
ALL_CAPS
while attributes should be insnake_case
, and for the most part, it looks like you're trying to conform to that, but some of your constants aren't... well constant. You defineself.API_URL
for example as the result of a variable passed to__init__
. This would mean that the value could conceivably be different in two instances of the same class. There's nothing wrong with that, but labelling it as if it were a constant is bad form.Type Hints
While not necessary for running your program, they're still super helpful both for readability and ensuring you haven't introduced any bugs accidentally. If you're using a proper IDE like PyCharm for example, it will warn you if you've passed an int to a method that's expecting a string for example. As your project gets more complicated, and you start passing around complex objects with varying abilities, type hints can be a big help.
Classes
Don't listen to the haters on classes. This is great. It's a good clean way to maintain state inside a component, rather than having to pass objects around to disparate functions.
The Superfluous
else
This is just a pet peeve of mine, but
else
should be used sparingly. There's nothing wrong with it per se, but it's often unnecessary. Consider the following two toy examples:That
else
is useless. the above could be:Similarly, this:
This would be a lot clearer if you invert the test:
This comes up all the time, and it just bugs me. I'm not throwing shade, just trying to reduce the number of superfluous else's in the world :-)
Complex dicts for data
The big offender here is
WeatherApp.LOCATIONS
. You define it first as a constant that you later change (see above), and you reference parts of it all over the place, meaning you've got to ensure that every part of your app (a) is familiar with its internals and (b) won't modify it, thereby breaking other parts of your app accidentally. There's also thisLOWLOCATIONS
business, which reads like a crutch you added to work around other limitations.Instead, I suggest delegating this sort of thing to a separate class. Consider something like this:
This would offload the complexity carrying around that structure throughout your application and instead define a set API between "how you think about locations" and "how your app uses them". It'd also let you remove a lot of your location-specific code from the rest of your project where arguably it doesn't belong.
Defining attritributes in
__init__
It's fine to change an instance's attributes from inside a random method, but initialising them should only ever happen in
__init__
. This is largely to do with readability, but also with stability. Consider the following:This will barf with an attribute error and when someone looks at the class trying to figure out where
alpha
is set,__init__()
is useless. Now imagine that the class is around 600 lines long and that's quite the headache.Sometimes, this is rather academic, like where you've called
.load_config()
from.__init__()
directly. In these cases, I'd recommend defining the attribute on the class so at least it's clear that this is something that the class supports externally:Delegation
It's great that you're trying to break up your project into responsible parts, but they're rather tightly coupled, so the responsibility to do anything rests with the entire app rather than any individual part.
For example, you've go this
WeatherAPI
class. It's a good idea. You want to delegate the responsibility of talking to an external API to something that knows how to do that (and only that), but in order to use the one you've built here, I'd need to pass it both the name of the city you want to look up, and the entire cities dataset, and the URL it needs to do the job.It's kind of like asking someone to make you a sandwich, and then taking the time to show them where the bread, ham, and cheese are in the fridge... you might as well make it yourself at that point.
Additionally, your API abstraction isn't just doing API things, but it's also handling the rendering step, doing a whole bunch of
print()
and colour formatting etc. That's not the job of an API.Instead, I suggest building this sort of thing "backwards". Decide on the API you want, and then build that out. Imagine for a moment how you might want the
WeatherAPI
class to work:This drastically simplifies your
WeatherAPI
class and allows you room to add support for things like caching, or handling rate limits etc. Better yet, you can delegate the various bits of data tweaking you're doing in bothWeatherApp
andWeatherAPI
to a separate class that just accepts raw API data and then distils it down to useful values. In the end, it might look something like this:That's it! Honestly, this is a good start, so please don't be discouraged by the verbosity of the above. Thanks for sharing your code, and have fun in future projects!
That's quite a detailed review. Thank you for your attention! I will try everything in here <3