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 types #21

Open
GideonBear opened this issue Feb 23, 2023 · 6 comments · May be fixed by #22
Open

Add types #21

GideonBear opened this issue Feb 23, 2023 · 6 comments · May be fixed by #22

Comments

@GideonBear
Copy link

Hi, thanks for the package! It would be great if type-hints could be added so this package can be used with mypy. Thanks!

@GideonBear
Copy link
Author

Since I don't think everyone wants to put assert everywhere, Any will have to be used a lot. But IMO, lots of Any is still better than no type-hints at all.
If you're okay with it, I can try to (partly) add types.

@controversial
Copy link
Owner

I’d welcome a PR!

@GideonBear
Copy link
Author

Hi @controversial, I think adding mypy to CI is a good idea, but how should I do that? I have no experience with Travis CI at all (I don't even see it running), but I can set up pre-commit or GitHub Actions if that's needed. (I'm assuming that's what you use, since there's a .travis.yml file)
Do you have any further preferences for mypy?

@GideonBear
Copy link
Author

Also, is python 2 (and python 3.6) support still required? The code could be updated to only support non-EOL versions, and it would mean we could type annotations instead of outdated comments.

@controversial
Copy link
Owner

I’m more than willing to drop support for EOL versions; I wrote this library a long time ago when Python 2/3.6 was still relevant!

@controversial
Copy link
Owner

Also, adding mypy to a Github Action is definitely a good idea; when I wrote this library Github Actions didn’t exist so Travis was a much more natural choice, but I wouldn’t choose it again today

@GideonBear GideonBear linked a pull request Mar 14, 2023 that will close this issue
27 tasks
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