Skip to content

ENH: Add some type annotations to play nicely with mypy #269

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

Merged
merged 4 commits into from
Oct 11, 2021

Conversation

headtr1ck
Copy link
Contributor

Fix for #268

Simply add type hints to LONG_VERSION_PY (use typing classes to support python<3.9)

@effigies effigies changed the title FIX: mypy fails after install ENH: Add some type annotations to play nicely with mypy Oct 5, 2021
@effigies
Copy link
Contributor

effigies commented Oct 5, 2021

I assume you've verified that this change resolves your issue?

@headtr1ck
Copy link
Contributor Author

Yes, I have tested it and mypy no longer complains.

It could be also possible to add tests to check the compatibility in the future.
Or even check mypy for the whole repo.

@effigies
Copy link
Contributor

effigies commented Oct 6, 2021

If it's easy to set up a mypy check in tox, feel free to propose it. I would be happy to include it in this PR or to merge this and have the mypy checks in a future PR.

@headtr1ck
Copy link
Contributor Author

Im a bit busy at the moment but would be happy to give it a try.
Should only files which are created be tested or the full repo?

@effigies
Copy link
Contributor

effigies commented Oct 7, 2021

If you check tox, we style check two generated files. Probably makes the most sense to test just those.

@headtr1ck
Copy link
Contributor Author

It seems that setuptools is lazy with type hints, so the verioneer.py file causes issues.
However, most projects will only check the src directory and not the root directory, so only checking the git_version.py is fine.

@headtr1ck
Copy link
Contributor Author

Should work like this, if wanted I can already start a Release 0.21 section in the NEWS.md and add this

@headtr1ck
Copy link
Contributor Author

Any idea why the setup fails?

@effigies
Copy link
Contributor

effigies commented Oct 8, 2021

mypy does not seem to install on pypy (python/typed_ast#111). I don't know if Tox will allow us to skip certain dependencies and test steps. Feel free to look into it, or I can try to find time this weekend.

Copy link
Contributor

@effigies effigies left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think I found the trick...

headtr1ck and others added 2 commits October 9, 2021 16:28
Co-authored-by: Chris Markiewicz <effigies@gmail.com>
Co-authored-by: Chris Markiewicz <effigies@gmail.com>
@effigies effigies merged commit 6a84ff7 into python-versioneer:master Oct 11, 2021
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.

2 participants