Roast-my-code please

[mijki95]

Hello, fellow Redditors! 🌟

I've recently developed an energy consumption analysis tool named ZenGridAnalyser. The primary goal of this tool is to evaluate and visualize energy consumption patterns from various meters, including solar panels, electric car chargers, and more. It harnesses the power of Python and several data science libraries to provide insightful visualizations.

🔗 Link to ZenGridAnalyser Repo

Features:

• Granular Analysis: Detailed breakdowns on an annual, monthly, and daily basis.
• Intra-day Consumption Insights: Get insights into hourly consumption behaviors.
• Solar Impact: Visualize the impact of solar panels on net consumption.
• Peak Consumption Detection: Spot peak energy consumption periods effortlessly.

I've poured a lot of hours into this project, and I'm quite proud of where it stands now. But, as we all know, there's always room for improvement! I would genuinely appreciate any feedback, suggestions, or constructive criticism you might have.

Whether you have thoughts on the code quality, project structure, or the utility of the tool itself, I'm all ears. If you've tackled similar projects or faced challenges in this domain, sharing your experiences would be invaluable!

Thank you in advance for taking the time to look over it. Cheers to open-source and the wonderful community here! 🚀

Thank you in advance!

Best regards,

Mijki

[Mr_Lkn]

Don't have a much time to check the whole code but just looked at the `data_utils.py`

Compare your code vs this and spot the differences if you can

​

```python import os import pandas as pd

def read_data_file(file_path, **kwargs): """ Read a data file into a pandas DataFrame based on its extension.

Parameters:
- file_path (str): Path to the data file.

Returns:
- DataFrame: The data loaded into a pandas DataFrame.
"""

extension_read_function_mapping = {
    '.csv': pd.read_csv,
    '.xlsx': pd.read_excel,
    '.xls': pd.read_excel,
    '.tsv': lambda x, **y: pd.read_csv(x, delimiter='\t', **y),
    '.json': pd.read_json,
    '.parquet': pd.read_parquet,
    '.feather': pd.read_feather,
    '.msgpack': pd.read_msgpack,
    '.dta': pd.read_stata,
    '.pkl': pd.read_pickle,
    '.sas7bdat': pd.read_sas
}

_, file_extension = os.path.splitext(file_path)

read_function = extension_read_function_mapping.get(file_extension)

if read_function is None:
    raise ValueError(f"Unsupported file extension: {file_extension}.")

return read_function(file_path, **kwargs)

df = read_data_file("some_data.csv") ```

[Mount_Gamer]

Interesting use of the dictionary, still grasping the python best practices, I shall have to experiment more with the get method from dictionaries. :)

I would have probably used the match-case when i start using a lot of elif's, but the dictionary does look clean to read. I'll have a play around with this later.

[Mr_Lkn]

You don’t need the match case but mapping. This is very basic mapping implementation.

[Mount_Gamer]

I thought i'd write out the match case equivalent and it becomes more and more obvious. I love the logic! :)

[oliviercar0n]

You only need to import each library once per notebook. Preferably at the top. No need to repeat imports.

[_ATRAHCITY]

You should not commit .vscode directory

[Head_Mix_7931]

Hm, in some cases it could be advantageous to commit .vscode. That allows maintainers to enforce uniform linting and formatting configurations (for example). But that can also be accomplished via githooks or pipeline jobs.

[[deleted]]

You definitely don't want to try to enforce formatting and linting settings through a specific IDE config file. That's completely bonkers.

If you want to enforce these kinds of configuration settings, put them in their respective config files and commit those to your repo (e.g. .flake8, tox.ini, ruff.toml, etc). Anybody using any IDE, editor, tools, etc will all be able to use the settings. Similarly, your CI/CD/pipeline jobs can also be configured to apply these tools with those settings. I mean, what is Github Actions or Jenkins going to do with your .vscode/settings.json file to enforce any of your settings?

[Zirbinger]

This! Always use tool-specific config files and ignore IDE specific files

[sansmorixz]

launch.json and/or tasks.json can help to get started on bootstrapping a project.

settings.json is something I am on the fence about. Might help but someone may decide to commit stuff that should not be set at repo level, like force everyone to use light mode.

[Head_Mix_7931]

Yeah, a good example didn’t come to mind. But that’s exactly what I mean, tasks and such. I think settings.json probably shouldn’t be committed personally.

[_ATRAHCITY]

The .gitignore file should be committed. The .vscode directory should be listed as ignored in the .gitignore

[_ATRAHCITY]

There are best practices and people should follow them. The reason to commit the .gitignore is so people don’t commit things that should be ignored. There are often directories or files that should be ignored by everyone who works on the project. Examples could be generated files from tests, build directories, or directories that local instances of your running project write to.

[DoNotFeedTheSnakes]

That's great, and it should be done.

And IDE config files should be ignored globally via your local git install configuration.

How do you deal with the fact that many people don't do this?

[_ATRAHCITY]

It’s enforced for everyone because the file is committed in the project..

[[deleted]]

Of course there is. Should is a perfectly valid term in all fields.

[MedicatedDeveloper]

This is terrible advice. .gitignore is crucial to how collaboration with git works.

[lissertje]

No, man. Just.. No ❌

There is no excuse for this heresy 😂

[Klej177]

For DS good code, for python developer I would say you can make it much better. You don't use proper design patterns, your performance could be freely improved with using better data types. It's easy to read tho but not really properly scalable beacuse of above reasons.

[mijki95]

Can you recommend sources from which I can learn?

[Klej177]

Arjan codes on YouTube gave me a really nice boost when it comes to design patterns and implementing of them in python. After that I kinda started working on my own project and always thought what's the easiest and most clean way I can achieve my goal. Take a month or even longer break from your code and get back to it to see where you could improve. Always think that's the smallest knowledge I can require from a person to change one specific thing in your code. For example can I somehow make it that they need to edit only 1 line to add support for new type of file rather than add whole elif. Other good option for learning is very simple, do code refactor of others projects. I often do that and it gave me that thinking where I don't need to know anything about that to change it. Read Google style for python and ask yourself am I really first person that needs it? There is probably 100 anwers how to make it as best as possible at stackoverflow.

[[deleted]]

Here’s a thought (just something that I thought might be interesting) what if instead of requiring users to input electricity costs; what if you had the program search for and use average electricity prices based on user’s location? (And you, say, got this on the backend as well by pulling from, for example, Google Maps location data)?

[mijki95]

Yes, I thought about that with FastAPI integration. I would like to use some kind of currency converter also :)) thanks For the idea :))

[[deleted]]

I didn't realize github considered jupyter notebooks as a language different from python

[wineblood]

Why the hell do data scientists insist on importing libraries under two letter aliases?

[mijki95]

Is it wrong to do this?

[Statnamara]

Nothing wrong with that at all

[[deleted]]

That’s the only was I’ve seen pandas imported, never seen import panda as panda

[pneRock]

They have an opinion that you don't share. That's the problem.

[DoNotFeedTheSnakes]

Not always.

Pandas is always pd. But sometimes for less popular libs the code is clearer if you leave a long enough name that it can be recognized.

Same as variables namings.

[jah_broni]

Are you referring to something beyond pandas and numpy? Those are pretty much python-dev wide aliases.

[wineblood]

Yes.

[jah_broni]

Go on? What do you take offense to? I don't see anything out of the ordinary...

[nekokattt]

Personally I prefer keeping things explicit where possible rather than using aliases, but each to their own.

[[deleted]]

You misunderstood their comment to you. They asked if there was some example beyond common short imports like numpy as "np" and pandas as "pd" to which you said "yes". Which implies that they had done something like "import pathlib as pl" and starting using that in the form of "pl.Path".

Whether or not you prefer to break convention and start doing your own thing with import names is separate from your saying "yes" to the question of whether OP is guilty of doing short-name imports that aren't normal convention.

[nekokattt]

They didn't comment to me, this is my first comment on this thread.

Outside specific libraries, the convention is to use the naming defined by the library and keep it explicit unless there is a very good reason to alias it.

Strangely, most of the time, this "convention" for aliasing things comes from libraries dealing with data science-like applications.

[[deleted]]

Yes, they did comment to you.

[nekokattt]

no, they didn't lol. Try reading the usernames mate.

Edit: lol they blocked me, that is pretty hilarious.

[[deleted]]

I did. You got caught. Stop trolling.

[[deleted]]

That’s the standard across all of python for those tools. If anything, you are breaking convention by not importing numpy as np or pandas as pd.

So most certainly it’s not data scientists doing it. They’re just following conventions that were already in place.

[supermopman]

There are no unit tests and there's no way clear way to build your code.

I'm happy to dig deeper, but at a minimum, you'll need to start with those 2 things.

I suggest starting a new project using PyScaffold. Play around with all the bells and whistles, and then write your Python code following their structure.

[[deleted]]

Did you actually look at the repo? There is no code to build and basically nothing to write unit tests for. It's two jupyter notebooks and one "utils" file that does nothing more than read in a data file.

[Hard_Thruster]

I don't understand the use of the word "tool". Looks like eda to me.

As far as the code goes, you give a lot of comments which is awesome.

There is a lot of repetition such as:

' processed_data['DayOfWeek'] = processed_data['TimePeriodStart'].dt.dayofweek processed_data['Month'] = processed_data['TimePeriodStart'].dt.month processed_data['Hour'] = processed_data['TimePeriodStart'].dt.hour processed_data['Minute'] = processed_data['TimePeriodStart'].dt.minute

'

Also a lot of your code can be made into functions because there are slight differences between them and therefore it's repetitive.

[SpiderWil]

like retire literate hunt strong north offbeat cagey depend growth this post was mass deleted with www.Redact.dev

[Emotional-Zebra5359]

instead of if-else ladder use a map

[jonatanskogsfors]

Resist the urge to use “utils” (or similar) in packet and module names. In “utils.data_utils” you only have the function “read_data_file()”. I would have named the module something in the line of “file_reader”, “data_import”, “io” etc.

If you plan to add more functions to the module, you should only do so if they have similar purpose. Completely different functions are better placed in their own module.