-
Notifications
You must be signed in to change notification settings - Fork 11
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
Add more type hints, fix some pylint warnings #71
Conversation
access_token = self.get_access_token() | ||
if not access_token: | ||
LOGGER.debug("%s Unable to obtain access token.", log_prefix) | ||
return None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if we should extract that check into a helper function decorator or just a helper function to call at all places as we do now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, those are some really good changes 💯
good-names = [ | ||
"zc", | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is that for? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To fix invalid-name
pylint warning in ServiceListener
methods. This allows zc
as a valid name.
@@ -279,6 +280,7 @@ def test_get_homegraph( | |||
self.assertEqual(m_get_home_graph_request.call_count, 1) | |||
|
|||
# Expired homegraph | |||
assert self.client.homegraph_date is not None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's have here unittest
pattern, just to have it consistent with the rest of the code?
assert self.client.homegraph_date is not None | |
self.assertIsNotNone(self.client.homegraph_date) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, mypy
can't narrow type from such calls: python/mypy#4063
Mypy helps to catch places where
Optional
is not handled properly.Fixes #66