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

Refactor code and fix imports #44

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from
Draft

Conversation

tabedzki
Copy link
Contributor

@tabedzki tabedzki commented Aug 1, 2024

Refactored the code to improve readability and removed unnecessary variables and expressions. Fixed incorrect variable references and organized imports and adjusted white space. This pull request aims to improve the overall code quality and maintainability.

Moving forward, we can use ruff to both lint and format our code.

@AlvaroS, can you give your thoughts on this

@Alvalunasan
Copy link
Contributor

Hi Christian, I think this is a great addition.
Do you have a plan on how we could test all changes introduced given the current state of the code?

@tabedzki
Copy link
Contributor Author

tabedzki commented Aug 7, 2024

I think the safest thing would be to setup a bunch of tests related to this. However, I think that would require a lot of work to verify that things like this work. There are options to make the formatter do only safe changes to the code, therefore it wouldn't do something like change

def f(a = []): pass # unsafe because every call to `f` would refer to the same list

into

def f(a=None):
    if a is None:
        a = [] # safe because the list gets assigned when the function is called and not when the function is defined
    pass

There is more that I would like to test out before making the plunge on this but having a formatter/linter seems like it would be useful.

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.

2 participants