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

Fix issue caused by having is_sign_message method and property of same name #578

Merged
merged 1 commit into from
Jul 19, 2024

Conversation

BamaHodl
Copy link
Contributor

@BamaHodl BamaHodl commented Jul 18, 2024

Description

Small fix of an issue caused by having a method and property of the same name. In scan_views.py, the check:
elif self.decoder.is_sign_message:
always passes the check because self.decoder.is_sign_message is of type method, evaluates to True because it's not None. It doesn't seem to be causing any issues currently because it's the final elif check for the QR type, however when adding new QR types below that elif, it will never flow to them. I experienced this when prototyping a new QR functionality. The following example code demonstrates the issue:

class TestClass:
    @property
    def test_it(self):
        return False

    def test_it(variable : str):
        return False

t = TestClass()
if not t.test_it:
    print ("Everything fine, got False for type " + str(type(t.test_it)))
else:
    print ("Not fine, wanted the property function but rather is of type " + str(type(t.test_it)))

Which yields:
Not fine, wanted the property function but rather is of type <class 'method'>

This pull request is categorized as a:

  • Bug fix

Checklist

  • I’ve run pytest and made sure all unit tests pass before sumbitting the PR

If you modified or added functionality/workflow, did you add new unit tests?

  • No, I’m a fool
  • Yes
  • N/A

I have tested this PR on the following platforms/os:

@dbast
Copy link
Contributor

dbast commented Jul 18, 2024

+1 nice finding... pyflakes (passive Python code checker) confirms the finding:

src/seedsigner/models/decode_qr.py:510:9: F811 Redefinition of unused `is_sign_message` from line 290
    |
509 |     @staticmethod
510 |     def is_sign_message(s):
    |         ^^^^^^^^^^^^^^^ F811
511 |         return type(s) == str and s.startswith("signmessage")
    |
    = help: Remove definition: `is_sign_message`

@newtonick
Copy link
Collaborator

ACK

@newtonick newtonick merged commit 5e4f669 into SeedSigner:dev Jul 19, 2024
2 checks passed
@BamaHodl BamaHodl deleted the MethodAndPropertyIssue branch July 20, 2024 15:10
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.

3 participants