-
Notifications
You must be signed in to change notification settings - Fork 114
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 type hints #118
Comments
This was primarily to maintain Py2 compatibility, but we should now be in the clear. All deprecation work is currently tracked in the https://github.com/kiwiz/gkeepapi/tree/deprecate_py2 branch. If you're feeling adventurous, PRs are welcome. :] |
Sure, I'll take a look at adding them. |
Let's use the latter to maximize compat. |
Sure, I'll do it. Could you please rebase the branch? |
Done. 👍 |
Thanks, pushed up fixes. |
Alright so instead of adding typing for everything all at once I'll focus on adding the initial Keep class typing, since that's the user-facing part. Some notes: In Keep::login we get an APIAuth object to login and then branch on its return value, but APIAuth::login always returns True if it doesn't raise a LoginException. We then return True for Keep::login as well. Is this intentional? Since Python doesn't have method overloading this update overrides this update. |
Hmm, these probably shouldn't return anything. In the interest of preserving backwards compat, let's not document the return value, but keep them for now. The
Good eye. The reminders functionality doesn't work atm, but I would rename the second |
Tracking in #130 |
Consider adding type hints so type checkers like mypy and on-the-fly documentation can work properly, at least for the most commonly used methods.
The return types are often already included in the documentation but aren't recognised correctly:
I'd expect something like
(method) get: (note_id) -> gkeepapi.node.TopLevelNode | None
insteadThe text was updated successfully, but these errors were encountered: