Code Issues I picked up(need help to understand!)

[im-coder-lg]

So, it has been a long time and I finally understand OOP. But I noticed some lengthy code and personally annoying things. So I want to speak about these issues I picked up, and want to know why it is so. This way, we could make this app better(and faster than it already is).

---

1. The weather data labels are pre-defined in the code. This makes the window larger but has no content. This seems like a bad UI choice, but I have a fix for it on my fork of this repo(basically replacing the .configure part in the update_labels method with the specific labels and frame in __init__ and changing the reference/call from update_labels to create-labels or create-data).

---

2. The use of PyOWM. It seems to be two requests.get at once. Let me explain:

I understand PyOWM as a module where it sends a request, gets the data back, and formats it for us. The data is initially in a dictionary, consisting of nested lists, sets and whatnot, and it is not only formatted but converted to various units. That is something we get from the raw method of requests.get and data formatting we used(data["main"]).
But why use PyOWM? I understand importing specific modules, but using requests.get and creating a method to transform raw data into refined data surely is a better choice. It could be specific for Imperial and Metric units, and using comprehension(* if "metric" else * where asterisks are data) is not entirely on the readability scale.
What I propose:
Remove PyOWM, and implement methods that transform the integers on its own. A bit complex to implement at face value, but it is easy to do so.

---

3. Too many imports. It seems a bit cluttered, is all. Even with formatting.
4. The ttk.Label part for the "Weather" text isn't centered properly.
5. The drop-down settings are too close to text and could use a separate window. Even though most are against that, that is what we see on many apps, plus, I want to let people use their tokens if they wish.

I might have more issues later, but let's try reasoning out on this and mutually agreeing on a fix.
Finally, this app could become something better than what it already is.

[im-coder-lg]

Sorry to disturb you @Moosems but you are the person that added more changes than me. Daresay, you added the MOST changes here. I need your ideas and opinions on this discussion.

I forked the repo to make my own changes. I know that I will get scathing reviews of my changes, but I wish to offer a link to a new thing I have going there: im-coder-lg#1

Please, I have very good reasons for this move.

[Moosems]

1. The weather data labels are pre-defined in the code. This makes the window larger but has no content. This seems like a bad UI choice, but I have a fix for it on my fork of this repo(basically replacing the .configure part in the update_labels method with the specific labels and frame in __init__ and changing the reference/call from update_labels to create-labels or create-data).

This is very intentional. It's generally considered bad UI and UX for the app to resize itself when it can be avoided. The update labels method was purpose made to be powerful yet simple.

2. The use of PyOWM. It seems to be two requests.get at once. Let me explain:

I understand PyOWM as a module where it sends a request, gets the data back, and formats it for us. The data is initially in a dictionary, consisting of nested lists, sets and whatnot, and it is not only formatted but converted to various units. That is something we get from the raw method of requests.get and data formatting we used(data["main"]).

But why use PyOWM? I understand importing specific modules, but using requests.get and creating a method to transform raw data into refined data surely is a better choice. It could be specific for Imperial and Metric units, and using comprehension(* if "metric" else * where asterisks are data) is not entirely on the readability scale.

What I propose:

Remove PyOWM, and implement methods that transform the integers on its own. A bit complex to implement at face value, but it is easy to do so.

No comment, I don't care too much how this is implemented.

3. Too many imports. It seems a bit cluttered, is all. Even with formatting.

I personally disagree with this. "Explicit is better than implicit". Star imports or somemodule.somefunction is never as good as somefunction(). The imports, when read by developers, are generally skipped anyway.

4. The ttk.Label part for the "Weather" text isn't centered properly.

That's not too difficult of a fix, but more a bug.

5. The drop-down settings are too close to text and could use a separate window. Even though most are against that, that is what we see on many apps, plus, I want to let people use their tokens if they wish.

A settings window could be arranged if we forced users to use a personal API token.

Finally, this app could become something better than what it already is.

I think it's simplicity is one of the things that makes it better than all the other ones out there.

[im-coder-lg]

For 3: It just confused me, but is becoming easier now. Anyways, can't we divide the processes into separate files? The configuration process can go to a config.py while the settings window can go to settings.py, so that expanding on these features can become easier.

[im-coder-lg]

1. So you say that the window expanding after a request is sent is considered bad UI/UX since it could be a hassle to users. Considering this, I wish to place the labels below the buttons.(How do the buttons look now?)

[im-coder-lg]

2. I can rectify this. It would add extra code, but I don't care, since everything is completed in one request. It might not be straightforward but it could be more effective.

[im-coder-lg]

Last one: yeah, and I'm trying to make it more simple, trust me. What do you think of adding the Mica effect here, like on good ol' TimerX?

[im-coder-lg]

4. We need to use columnspan=2 after creating a settings window.

5. We don't need to force users yet. Let's add the tokens system, make sure that actually works, and then we remove the token in the code.

[Moosems]

In general, when proposing changes, it's also better to make a lot of small PRs than one big one. It's far nicer to review a few than one massive one.

[im-coder-lg]

It won't be massive, trust me. All I am fixing are inconsistent things I noticed.

[im-coder-lg]

Okay, so I must fix the labels part on my fork. But I'm keeping the settings window and adding the tokens dialog.

And I centred the Weather label. Just added columnspan=2 after moving the drop-downs to the settings window.

And how do the buttons look?

[sumeshir26]

@Futura-Py/reviewers Damm guys its been a while 😭
Unfortunately couldnt/didnt want to do anything on GitHub for a while so sorry if I missed anything :(
tho it dosent seem like much has happened 💀

[sumeshir26]

But while nothing happened here after me and Moosems tried and added some stuff, a while lot of things have happened in other repos and the world. For starters, I am re-learning Python at school. Anybody doing that here?

My shit-ass school CS course is teaching us C++ in Visual Studio 2012

And as I said, I have ideas. Starting with a sidebar. A collapsible sidebar.

Any ideas? With our time constraints, we have to plan for one feature at a time, and perhaps then everything will work.

Feel free to go ahead but ig i wont be able to contribute...

And I finally found a reason for Weather to exist. Literally thought of this just now after daily driving Windows 11. Windows has a really bad Weather app. And this app can change that. No ads, and only the data you want. All in a simple client.

Agreed.

[im-coder-lg]

My shit-ass school CS course is teaching us C++ in Visual Studio 2012

I don't condone cursing, but I agree. A Udemy course can do better. And Python is already part of the Indian Government's CBSE CS books. It doesn't add up.
And they are making you use VS 2012. A newer version can be installed for this IG, and anyway, the IDE and modules could change, not C++.

Feel free to go ahead but ig i wont be able to contribute...

It's fine. Do what you can. Even I don't have time to code so I get it. The coming holidays should help me get time for this though.

[HuyHung1408]

For starters, I am re-learning Python at school. Anybody doing that here?

Same with my school, but we went through basic stuffs last year. This year we gonna learn some Computer Science-related, this one is compulsory for everyone in class tho, even for those who are not interested in coding. Just saying hi and goodluck to yall.

[im-coder-lg]

Thanks man, and all the best!

[sumeshir26]

Thanks dudee

[im-coder-lg]

@Futura-Py/reviewers is this gonna affect us for Weather?

I think we use One Call API 2.5, and on Sep 17, it will be deprecated.

[im-coder-lg]

upd: we can't use the city variable like we used to ig

normal method gone, carry on

edit: that's with one call 3.0 and I don't think that would affect us badly.

[im-coder-lg]

Alright, I'm leaving it here. If we should change it, we will.

[Moosems]

@im-coder-lg The token is already out there. What you need to do is set a charge limit with them so that you never are given responses after the 1000 free ones.

[im-coder-lg]

The free tier allows 60 calls per minute and has nifty things like geolocating for latitudes and longitudes. But the token we use here belongs to @sumeshir26 (correct me if I'm wrong). Is it a free-tier plan? If yes, I just got more ideas for this, like search recommendations if multiple places exist with the same name. We can add this idea to the app only if we need it.
And why not a sidebar, and saved locations? Those seem good, and we can make them into buttons. By adding it into a list in a config JSON dictionary, every time the app loads, it loads the JSON config, and thus loads the list of saved locations, and thus loads the place's weather data. How does this sound?

[sumeshir26]

Yup its my token... The sidebar seems helpful too.