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

Add Type Hints #137

Closed
2e0byo opened this issue May 28, 2023 · 4 comments · Fixed by #196
Closed

Add Type Hints #137

2e0byo opened this issue May 28, 2023 · 4 comments · Fixed by #196
Assignees

Comments

@2e0byo
Copy link
Collaborator

2e0byo commented May 28, 2023

Part of the intention of dropping support for python 2.7 was being able to use type hints. This would help with tehkillerbee/mopidy-tidal#112 as well.

@2e0byo 2e0byo self-assigned this May 28, 2023
@2e0byo
Copy link
Collaborator Author

2e0byo commented May 28, 2023

[self-assigned but feel free to do it for me ;) ]

@arusahni
Copy link
Contributor

arusahni commented May 28, 2023

Would you accept this as a series of smaller PRs, or would you prefer it land in one fell swoop? I figure incrementally introducing typing will be easier, but that's based on a few minutes of poking around as I integrated something with this API library.

@2e0byo
Copy link
Collaborator Author

2e0byo commented May 29, 2023

Would you accept this as a series of smaller PRs?

Definitely. I'd do it in one swoop if I sat down to do it, but anything helps. In my experience 80-90% of type hinting is low hanging fruit, and then you hit the remaining code where you either have to do something inventive or consider tweaking the implementation. So if that turns out to be the case here, we should do the low hanging fruit first so people can start benefitting.

@arusahni
Copy link
Contributor

Thanks! I put up an initial PR to get the baseline stuff out of the way (type checking, config, etc.). Once we're in agreement, I can start landing the bigger pieces of API surface.

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