-
-
Notifications
You must be signed in to change notification settings - Fork 906
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
Type Hints #1095
Comments
I am absolutely open to receiving PRs adding type hints, and GitPython would be happy to host them directly in source. Any amount of types would help and is appreciated, thus there is no need to add type hints everywhere at once. |
I looked at starting to contribute type hints, but wanted to ask would it be feasible to drop support for python 3.4 (EOL 2 years ago)? With 3.5+ type hints can be added directly to the code, but for 3.4 they would need to be in .pyi files, doubling the number of files and requiring they are kept in sync (AFAIK, 3.4 will give errors rather than ignore most inline type hints even though annotation syntax is possible) |
@Yobmod Thanks for bringing this to my attention, and I would be glad to see type hints in code. Please feel free to submit a draft PR that starts out by dropping PY3.4 support to allow adding the first type hints in code. That one wouldn't have to be complete at all, but could lay the foundation for more PRs adding types in future. |
I've put in a PR to drop python 3.4. |
A plan:
|
Thank for all the effort towards type hints! I am sorry I have not been able to contribute so far, I hope to be able to put some effort after the great initial start of @Yobmod |
Thanks, Any help is always good :) Branch for adding types is here: https://github.com/Yobmod/GitPython/tree/addtypes Mypy-stubgen just typed everythin as Any, and Monkeytype has not been as useful as i hoped. If any wants to run it, commands are:
But i'm mostly doing them by hand. |
Hey, it seems that most of the code is typed, right? Would it be possible to include a "py.typed" file into the repo so that mypy can pick the types up? |
Hi, there already is one. Are you on the newest version? |
Yes I use the latest version. The py.typed file is not present in the tar.gz nor in the wheel - maybe that's the issue. |
I've read up on some setuptools options, it seems that the package_data and include_package_data arguments are mutually exclusive. So setup.py is picking up the package_data only from the MANIFEST.INI and not from within the setup.include_package_data list. I'll add py.typed to the manifest and hopefully that fixes it. |
Great. thanks a lot |
I've installed from git and it puts the py.typed where it should be now.
|
@Yobmod thanks for the work here. Are there any plans when 3.1.19 will be released? |
Yep, we could do a release now. The next step for typing is to check the runtime types to fix the remaining unknowns. Unfortunately I cant find a library to do it (Typeguard etc. will report if using the wrong type, but not if the type is too broad), so it will take a while. |
And here is the new release. Fingers crossed! |
@Byron Just got this with 3.1.19 and Python 3.9.6. Maybe we should yank it? |
It looks like typing_extensions has to be updated (again) to 3.10.0.0.
That fixed it on my machine, so I've push the commit for requirements.txt.
Typehints still seem to be changing every few months, even after 7 years!
Dominic
…On Fri, 23 Jul 2021, 04:45 Anderson de Andrade, ***@***.***> wrote:
import git
File "/opt/python/lib/python3.9/site-packages/git/__init__.py", line 8, in <module>
from git.exc import * # @nomove @IgnorePep8
File "/opt/python/lib/python3.9/site-packages/git/exc.py", line 10, in <module>
from git.compat import safe_decode
File "/opt/python/lib/python3.9/site-packages/git/compat.py", line 32, in <module>
from git.types import TBD
File "/opt/python/lib/python3.9/site-packages/git/types.py", line 18, in <module>
from typing_extensions import TypeGuard # noqa: F401
ImportError: cannot import name 'TypeGuard' from 'typing_extensions' (/opt/python/lib/python3.9/site-packages/typing_extensions.py)
@Byron <https://github.com/Byron> Just got this with 3.1.19 and Python
3.9.6. Maybe we should yank it?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1095 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AFJUSQ3XHNB6XABCFOWJSUDTZDQUVANCNFSM4VD23UVQ>
.
|
Hello, Here the content of the installation folder:
type hints started to work after a |
Interesting this wasn’t caught by CI.
Yanked the release.
…Sent from my iPhone
On Jul 23, 2021, at 5:17 PM, Dominic ***@***.***> wrote:
It looks like typing_extensions has to be updated (again) to 3.10.0.0.
That fixed it on my machine, so I've push the commit for requirements.txt.
Typehints still seem to be changing every few months, even after 7 years!
Dominic
On Fri, 23 Jul 2021, 04:45 Anderson de Andrade, ***@***.***>
wrote:
> import git
> File "/opt/python/lib/python3.9/site-packages/git/__init__.py", line 8, in <module>
> from git.exc import * # @nomove @IgnorePep8
> File "/opt/python/lib/python3.9/site-packages/git/exc.py", line 10, in <module>
> from git.compat import safe_decode
> File "/opt/python/lib/python3.9/site-packages/git/compat.py", line 32, in <module>
> from git.types import TBD
> File "/opt/python/lib/python3.9/site-packages/git/types.py", line 18, in <module>
> from typing_extensions import TypeGuard # noqa: F401
> ImportError: cannot import name 'TypeGuard' from 'typing_extensions' (/opt/python/lib/python3.9/site-packages/typing_extensions.py)
>
> @Byron <https://github.com/Byron> Just got this with 3.1.19 and Python
> 3.9.6. Maybe we should yank it?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <#1095 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AFJUSQ3XHNB6XABCFOWJSUDTZDQUVANCNFSM4VD23UVQ>
> .
>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
😭 Encode/starlette have py.typed listed in setup.py package_data, but not in manifest.in, and that works. Encode/httpx (issue linked above) had same issue, so added py.typed to manifest.in. I've now added it to both setup.py and manifest.in (DRY is so 1980s) to see if that fixes it (based on https://stackoverflow.com/questions/3596979/manifest-in-ignored-on-python-setup-py-install-no-data-files-installed) If that doesn't work, the only thing I can think of is something to do with the installed lib having a different name? (git vs gitpython). Maybe should release a beta with the fixes I commited this morn? Then I can install from pypi. I can't think of how else to test this? |
Yeah, CI doesn't error because it only happens with pypi install into already existing environments, but CI is making fresh venvs with the newest available libs each time |
mypy docs (see the very last paragraph in https://mypy.readthedocs.io/en/latest/installed_packages.html#creating-pep-561-compatible-packages) suggests:
in MANIFEST.in But i think that (However, none of the files in the MANIFEST.in are included in the package... it seems that the MANIFEST is ignored at all) |
This release includes the unfinished addition of types to the library. For more information see: gitpython-developers/GitPython#1095
I think the more version specific guard is the correct fix, mypy just complaining because it can't cope with the same variable being defined 3 different ways for different pythons. |
What is the gitobj? For some of the Git comand wrappers, they are too dynamic to add types (so default to Any), until/unless someone adds specific methods/overloads for each command. |
gitobj is an instance of Repo Here a minimal test case to reproduce the issue:
|
Ah, no error for me. What does mypy show for reveal_type(gitobj.active_branch)? So maybe adding type hint to symbolicReference.name has fixed it in main? (Head <- Reference <- SymbolicReference) |
|
Thx for checking. Then it is fixed for next release :) |
Thanks everyone for bearing with us. Yesterday after the second issue was filed I yanked 3.1.20, so it looks like that the issues around older python versions and ordered dict is all there is for now. @Yobmod What do you think about setting up CI to test patch releases as well, to have more probes when determining that the type related changes are actually working for everyone? Typing seems to be a minefield when trying to support as many python versions as we do, and I have a feeling we have to beef up our testing game in order to avoid all these hassles in the future. |
Hi all, just a short idea: typing imports could be wrapped into In addition there is also a checker for flake8: https://github.com/asottile/flake8-typing-imports foo.py from typing import OrderedDict Execute:
EDIT: I'm happy to file a PR to add flake8-typing-imports if you think this makes sense |
When running in strict mode one gets failures for the repository not exporting explicitly its properties:
See python/mypy#10198 (comment) for more details. |
We already have a similar library in requirements-dev.txt (flake8-type-checking), but it has too many false positives to use in CI. The issue for Orderdict though is that we are subclassing it, so one import has to be runtime available. Maybe we can put the typed versions in the |
Yep, we have a lot of * imports that rely on implicit-reexport for mypy. For end-users this should not cause problems, but its a known mypy bug: python/mypy#8754. If someone wants to change all the * imports to match |
I personally think for reasons explained in python/mypy#10198 (comment) this is not a mypy bug 🤔 and I don't think due to that will be fixed 🤔 At best I'd imagine in |
Yes, I guess it will only prevent runtime issues but during type checking time, the imports must work. However, normal users might get more stable releases
Let me check and open a PR if possible. Please note, that these libraries are doing different things. The current one is more style related, while flake8-typing-imports checks import from typing based on the current python version |
So, I opened #1309. |
Seems useful already! So we have to decide:
(NoReturn and Deque are available in typing_extensions, so that can be easily fixed.) I was planning to wait until 3.10 released to drop 3.6. |
Dropping py 3.6 seems like the path of least resistance. |
See python/mypy#8754 (comment), seems this is not a bug in mypy, but a feature and the issue lies in this package, not the type checkers (both mypy and pyright seem to assume that implicit re-export is off). |
Its fixable here, but there are 156 implicit-reexports errors if its activated in the codebase atm, so it wont be done for the next few releases unless someone volunteers. |
Is there any update how you want to proceed with this? |
I think i've done everything I planned for the next release. Testpypi uploads fine (using 3.7.3), so ready to go! |
Thanks a lot! Thanks to a 'hickup' this was ultimately released as https://pypi.org/project/GitPython/3.1.23/ . |
Hello,
I was looking for any type hints for GitPython, but I found nothing neither in the code nor in independent stub repositories.
Is there any plan to introduce type hinting?
The text was updated successfully, but these errors were encountered: