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

Pep8 violation in the code #101

Open
Drachenfels opened this issue Apr 19, 2024 · 6 comments · May be fixed by #102
Open

Pep8 violation in the code #101

Drachenfels opened this issue Apr 19, 2024 · 6 comments · May be fixed by #102
Assignees

Comments

@Drachenfels
Copy link
Collaborator

In some places, you have syntax like this:

if not xalign == None:

It's actually against Python coding standard -> https://stackoverflow.com/questions/14247373/python-none-comparison-should-i-use-is-or

Use either:

xalign is None

or

xalign is not None

Statement:

not xalign == None

Is a double whammy as it's using a double negation, and it's hard to follow when it's True and when it's False, imagine those cases:

not "" == None -> True
not None == None -> False
not 0 == None -> True
not 1 == None -> True
not False == None -> True

So perhaps simpler:

xalign is not None

would suffice?

@BlackRam-oss
Copy link
Collaborator

yes you are right this change should be made, I created a linked branch.
if you want you can do it yourself.

@Drachenfels
Copy link
Collaborator Author

Sure thing, will raise PR for you, do you want me to change things in more places as well? Or as little as possible?

@BlackRam-oss
Copy link
Collaborator

BlackRam-oss commented Apr 19, 2024

if you want throughout the project. the important thing is that you test that everything works. For dowload the project: https://github.com/DRincs-Productions/NQTR-System?tab=readme-ov-file#to-download-this-test-project

@Drachenfels
Copy link
Collaborator Author

Sure thing, I will work on it either tomorrow or Wednesday. Will use black or some other linter to check all the python code.

@Drachenfels
Copy link
Collaborator Author

FYI:

class Button(DisabledClass):
    def __init__(self):
        self.align = None
    
    @property
    def align(self) -> Optional[tuple[Union[int, float], Union[int, float]]]:
        """X align"""
        return self._align

Is incorrect.

It works by accident, you change align first to be None and then you change it to be property by side effect of calling this: self.xalign = 0

@Drachenfels
Copy link
Collaborator Author

I wrote this simple test to showcase it:

def test_align_not_working_as_expected():
    from pythonpackages.nqtr.button import Button

    some_button = Button()

    assert some_button.align is None

    some_button.xalign = 0

    assert type(some_button.align) is tuple
    assert some_button.align == (0, 0)

@Drachenfels Drachenfels linked a pull request May 7, 2024 that will close this issue
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 a pull request may close this issue.

2 participants